-
Notifications
You must be signed in to change notification settings - Fork 264
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add support for maxItemsComputed #444
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -34,8 +34,11 @@ export function getFoldingRanges(document: TextDocument, context: FoldingRangesC | |
if (typeof rangeLimit !== 'number' || result.length <= rangeLimit) { | ||
return result; | ||
} | ||
if (context && context.onRangeLimitExceeded) { | ||
context.onRangeLimitExceeded(document.uri); | ||
} | ||
|
||
return result.slice(0, context.rangeLimit - 1); | ||
return result.slice(0, context.rangeLimit); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe this was an off-by-one error that I've now added a test for. |
||
} | ||
|
||
function creteNormalizedFolding(document: TextDocument, node: ASTNode): FoldingRange { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,13 +19,29 @@ describe('Document Symbols Tests', () => { | |
let languageHandler: LanguageHandlers; | ||
let yamlSettings: SettingsState; | ||
|
||
const limitContent = ` | ||
a: [1, 2, 3] | ||
b: [4, 5, 6] | ||
`; | ||
|
||
before(() => { | ||
const languageSettingsSetup = new ServiceSetup(); | ||
const { languageHandler: langHandler, yamlSettings: settings } = setupLanguageService(languageSettingsSetup.languageSettings); | ||
languageHandler = langHandler; | ||
yamlSettings = settings; | ||
}); | ||
|
||
afterEach(() => { | ||
yamlSettings.maxItemsComputed = 5000; | ||
}); | ||
|
||
function assertLimitWarning(): void { | ||
const warnings = languageHandler.pendingLimitExceededWarnings; | ||
assert.deepEqual(Object.keys(warnings), [TEST_URI]); | ||
assert.deepEqual(warnings[TEST_URI].features, { 'document symbols': 'document symbols' }); | ||
assert(warnings[TEST_URI].timeout); | ||
} | ||
|
||
describe('Document Symbols Tests (Non Hierarchical)', function () { | ||
function parseNonHierarchicalSetup(content: string): SymbolInformation[] | DocumentSymbol[] { | ||
const testTextDocument = setupTextDocument(content); | ||
|
@@ -121,6 +137,16 @@ describe('Document Symbols Tests', () => { | |
assert.deepEqual(symbols[0], createExpectedSymbolInformation('analytics', 17, '', TEST_URI, 1, 0, 1, 15)); | ||
assert.deepEqual(symbols[1], createExpectedSymbolInformation('json', 15, '', TEST_URI, 4, 0, 4, 10)); | ||
}); | ||
|
||
it('Document symbols with a limit', () => { | ||
yamlSettings.maxItemsComputed = 1; | ||
|
||
const symbols = parseNonHierarchicalSetup(limitContent); | ||
assert.equal(symbols.length, 1); | ||
assert.deepEqual(symbols[0], createExpectedSymbolInformation('a', SymbolKind.Array, '', TEST_URI, 1, 4, 1, 16)); | ||
|
||
assertLimitWarning(); | ||
}); | ||
}); | ||
|
||
describe('Document Symbols Tests (Hierarchical)', function () { | ||
|
@@ -275,5 +301,21 @@ describe('Document Symbols Tests', () => { | |
createExpectedDocumentSymbol('conditions', SymbolKind.Module, 6, 12, 10, 28, 6, 12, 6, 22, [root2]) | ||
); | ||
}); | ||
|
||
it('Document symbols with a limit', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why does this test and the one above on line 141 have the same description? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
These are for the hierarchical and non-hierarchical cases inside separate describe blocks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, thanks. I missed that in the truncated github diff view. |
||
yamlSettings.maxItemsComputed = 3; | ||
|
||
const symbols = parseHierarchicalSetup(limitContent) as DocumentSymbol[]; | ||
assert.equal(symbols.length, 2); | ||
assert.equal(symbols[0].children.length, 1); | ||
assert.equal(symbols[1].children.length, 0); | ||
|
||
const el = createExpectedDocumentSymbolNoDetail('0', SymbolKind.Number, 1, 8, 1, 9, 1, 8, 1, 9, []); | ||
const root = createExpectedDocumentSymbol('a', SymbolKind.Array, 1, 4, 1, 16, 1, 4, 1, 5, [el]); | ||
|
||
assert.deepEqual(symbols[0], root); | ||
|
||
assertLimitWarning(); | ||
}); | ||
}); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There weren't any existing tests for
foldingRangeHandler
so I didn't add any to test this new functionality (just the underlyinggetFoldingRanges
).