From 8799fb053b25d51905eef0a6960d27ed565c7be2 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sun, 14 Jan 2024 09:51:08 -0800 Subject: [PATCH 1/9] Flesh out service documentation --- packages/symbols-view/lib/main.d.ts | 41 ++++++++++++++++++++++++++--- 1 file changed, 38 insertions(+), 3 deletions(-) diff --git a/packages/symbols-view/lib/main.d.ts b/packages/symbols-view/lib/main.d.ts index 580fd8e6f..40f142537 100644 --- a/packages/symbols-view/lib/main.d.ts +++ b/packages/symbols-view/lib/main.d.ts @@ -228,7 +228,9 @@ export interface SymbolProvider { // // This method receives the same metadata bundle that will be present in the // subsequent call to `getSymbols`. The provider can inspect this metadata - // and decide whether it can fulfill the given symbols request. + // and decide whether it can fulfill the given symbols request. It _should + // not_ start the task of gathering symbols; the point of this method is to + // determine which provider is best for the task without starting the work. // // Examples: // @@ -239,10 +241,16 @@ export interface SymbolProvider { // the current unsaved contents of buffers, could return a slightly lower // score if asked to complete symbols for a file that has been modified. // This would indicate that it’d be a slightly worse than usual candidate. + // * If my provider can do project-wide symbol search but _can't_ do a + // go-to-definition lookup, it can still serve as a fallback provider when + // `type` is `project-find`. But it should return a lower score to reflect + // that it's not the ideal choice. // // Since language server providers will have to ask their servers about // capabilities, this method can go async, though it’s strongly suggested to - // keep it synchronous if possible. + // keep it synchronous if possible. (The `timeoutMs` argument isn't yet + // enforced on `canProvideSymbols`, but we reserve the right to start + // enforcing it at any point without a bump in the service's version number.) // // To avoid a number war, any numeric value greater than `1` returned from // `canProvideSymbols` will be clamped to `1`. The user can break ties by @@ -262,13 +270,40 @@ export interface SymbolProvider { // // This method can go async if needed. Whenever it performs an async task, it // should check `meta.signal` afterward to see if it should cancel. + // + // The `type` property of `meta` affects which symbols this method should + // return: + // + // * If `type` is `file`, this method should return a full list of symbols + // for the current file. + // + // * If `type` is `project`, this method should return an _appropriate_ list + // of symbols for the project. The ideal approach would be to return only + // those results that match `meta.query`; you may choose not to return any + // symbols at all until `meta.query` is of a minimum length. But you may + // also return a full list of project symbols and rely on `symbols-view` to + // do all of the filtering as the user types. (In the latter case, + // `getSymbols` will still be called after each new keystroke; a future + // version of this service may offer a way to control that behavior.) + // + // If you return an empty list when `meta.query` is too short, you should + // use `listController` to set a message in the UI so that users understand + // why. + // + // * If `type` is `project-find`, the user is trying to find where + // `meta.query` is defined (a go-to-definition request). If this provider + // knows how to do that, it should find the answer and return it as the + // _only_ symbol in the returned list. If it doesn't, it is allowed to + // treat this similarly to a project-wide symbol search and return more + // than one result. + // getSymbols(meta: FileSymbolMeta, listController?: ListController): MaybePromise getSymbols(meta: ProjectSymbolMeta, listController?: ListController): MaybePromise } export type SymbolProviderMainModule = { activate(): void, - deacivate(): void, + deactivate(): void, // No business logic should go in here. If a package wants to provide symbols // only under certain circumstances, it should decide those circumstances on From 33b4eea80d4015a80bf473726f64fa9a7c8527af Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sun, 14 Jan 2024 09:51:31 -0800 Subject: [PATCH 2/9] Align icons better alongside symbol names --- packages/symbols-view/styles/symbols-view.less | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/symbols-view/styles/symbols-view.less b/packages/symbols-view/styles/symbols-view.less index 7e1a7151c..da33cbeea 100644 --- a/packages/symbols-view/styles/symbols-view.less +++ b/packages/symbols-view/styles/symbols-view.less @@ -43,7 +43,11 @@ atom-panel.modal .symbols-view { .primary-line { display: flex !important; flex-direction: row; - align-items: baseline; + align-items: center; + + &.icon::before { + margin-left: 0; + } .name { text-overflow: ellipsis; From 58d9a0393ecc16f543538d61201757ad0d8ef429 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sun, 14 Jan 2024 10:09:21 -0800 Subject: [PATCH 3/9] [symbol-provider-tree-sitter] Fix icon display via `symbol.icon` Specifying the icon via the `(#set! symbol.icon "foo")` predicate was having no effect. --- packages/symbol-provider-tree-sitter/README.md | 2 ++ .../symbol-provider-tree-sitter/lib/capture-organizer.js | 6 +++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/symbol-provider-tree-sitter/README.md b/packages/symbol-provider-tree-sitter/README.md index 0d2d00567..9ef72458b 100644 --- a/packages/symbol-provider-tree-sitter/README.md +++ b/packages/symbol-provider-tree-sitter/README.md @@ -179,3 +179,5 @@ If this value is omitted, this provider will still attempt to match certain comm The `symbol.tag` predicate will set the value of a symbol’s `tag` property to a fixed string. The `tag` property is used to supply a word that represents the symbol in some way. For conventional symbols, this will often be something like `class` or `function`. + +This provider will attempt to match certain common tag values to icons. This can be overridden by specifying an explicit `symbol.icon` value. diff --git a/packages/symbol-provider-tree-sitter/lib/capture-organizer.js b/packages/symbol-provider-tree-sitter/lib/capture-organizer.js index ef7a926f7..9086bb149 100644 --- a/packages/symbol-provider-tree-sitter/lib/capture-organizer.js +++ b/packages/symbol-provider-tree-sitter/lib/capture-organizer.js @@ -246,11 +246,11 @@ class Name { } toSymbol() { - let { name, shortName, position, context, tag } = this; - let symbol = { name, shortName, position }; + let { name, shortName, position, context, tag, icon } = this; + let symbol = { name, shortName, position, icon }; if (tag) { symbol.tag = tag; - symbol.icon = iconForTag(tag); + symbol.icon ??= iconForTag(tag); } if (context) symbol.context = context; From 3565ea822e6968a31610fd21e922cc7834e0a662 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sun, 14 Jan 2024 13:38:33 -0800 Subject: [PATCH 4/9] [symbol-provider-tree-sitter] Add specs for `context`, `tag`, `icon` --- .../tree-sitter/tree-sitter-markdown/tags.scm | 24 ++- .../symbol-provider-tree-sitter/README.md | 34 ++-- .../lib/capture-organizer.js | 86 +++++++--- .../spec/symbol-provider-tree-sitter-spec.js | 160 ++++++++++++++++++ 4 files changed, 262 insertions(+), 42 deletions(-) diff --git a/packages/language-gfm/grammars/tree-sitter/tree-sitter-markdown/tags.scm b/packages/language-gfm/grammars/tree-sitter/tree-sitter-markdown/tags.scm index de77c5828..b77668759 100644 --- a/packages/language-gfm/grammars/tree-sitter/tree-sitter-markdown/tags.scm +++ b/packages/language-gfm/grammars/tree-sitter/tree-sitter-markdown/tags.scm @@ -3,46 +3,54 @@ (atx_h1_marker) (heading_content) @name) @definition.heading (#set! symbol.strip "(^\\s*|\\s*$)") - (#set! symbol.prepend "· ")) + (#set! symbol.prepend "· ") + (#set! symbol.icon "chevron-right")) ((atx_heading (atx_h2_marker) (heading_content) @name) @definition.heading (#set! symbol.strip "(^\\s*|\\s*$)") - (#set! symbol.prepend "·· ")) + (#set! symbol.prepend "·· ") + (#set! symbol.icon "chevron-right")) ((atx_heading (atx_h3_marker) (heading_content) @name) @definition.heading (#set! symbol.strip "(^\\s*|\\s*$)") - (#set! symbol.prepend "··· ")) + (#set! symbol.prepend "··· ") + (#set! symbol.icon "chevron-right")) ((atx_heading (atx_h4_marker) (heading_content) @name) @definition.heading (#set! symbol.strip "(^\\s*|\\s*$)") - (#set! symbol.prepend "···· ")) + (#set! symbol.prepend "···· ") + (#set! symbol.icon "chevron-right")) ((atx_heading (atx_h5_marker) (heading_content) @name) @definition.heading (#set! symbol.strip "(^\\s*|\\s*$)") - (#set! symbol.prepend "····· ")) + (#set! symbol.prepend "····· ") + (#set! symbol.icon "chevron-right")) ((atx_heading (atx_h6_marker) (heading_content) @name) @definition.heading (#set! symbol.strip "(^\\s*|\\s*$)") - (#set! symbol.prepend "······ ")) + (#set! symbol.prepend "······ ") + (#set! symbol.icon "chevron-right")) ((setext_heading (heading_content) @name) @definition.heading (setext_h1_underline) (#set! symbol.strip "(^\\s*|\\s*$)") - (#set! symbol.prepend "· ")) + (#set! symbol.prepend "· ") + (#set! symbol.icon "chevron-right")) ((setext_heading (heading_content) @name) @definition.heading (setext_h2_underline) (#set! symbol.strip "(^\\s*|\\s*$)") - (#set! symbol.prepend "·· ")) + (#set! symbol.prepend "·· ") + (#set! symbol.icon "chevron-right")) diff --git a/packages/symbol-provider-tree-sitter/README.md b/packages/symbol-provider-tree-sitter/README.md index 9ef72458b..e5a5dc204 100644 --- a/packages/symbol-provider-tree-sitter/README.md +++ b/packages/symbol-provider-tree-sitter/README.md @@ -107,7 +107,7 @@ This allows us to incorporate any transformations that were applied to the other ##### Adding the `context` field -The `context` field of a symbol is a short piece of text meant to give context. For instance, a symbol that represents a class method could have a `context` field that contains the name of the owning class. The `context` field is not filtered on. +The `context` field of a symbol is a short piece of text meant to give context. For instance, a symbol that represents a class method could have a `context` field that contains the name of the class it belongs to. The `context` field is not filtered on. ###### symbol.contextNode @@ -135,7 +135,11 @@ The point of `context` is to provide information to help you tell symbols apart, ##### Adding a tag -The `tag` field is a string (ideally a short string) that indicates a symbol’s kind or type. A `tag` for a class method’s symbol might say `method`, whereas the symbol for the class itself might have a `tag` of `class`. These tags will be indicated in the UI with a badge or an icon. +The `tag` field is a string that indicates a symbol’s kind or type. It should be a single word wherever possible. A `tag` for a class method’s symbol would typically be `method`, whereas the symbol for the class itself would typically have a `tag` of `class`. These tags will be indicated in the UI with a badge, an icon, or both. + +If you’re not sure what to call something, consult [this list from the Language Server Protocol spec](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#symbolKind). But some symbols may not fit any of those, so ultimately it’s up to the author. (For example, headings in Markdown files are assigned a kind of `heading`.) + +For consistency, tags should be all lowercase. The interface will apply its own casing effect through CSS (`text-transform: capitalize` by default, but customizable in UI themes). The preferred method of adding a tag is to leverage the `@definition.` captures that are typically present in a tags file. For instance, in this excerpt from the JavaScript grammar’s `tags.scm` file… @@ -154,18 +158,7 @@ The preferred method of adding a tag is to leverage the `@definition.` captures In cases where this is impractical, you can provide the tag explicitly with a predicate. -###### symbol.icon - -```scm -(class_body (method_definition - name: (property_identifier) @name - (#set! symbol.icon "package") -)) -``` - -The icon to be shown alongside the symbol in a list. Will only be shown if the user has enabled the “Show Icons in Symbols View” option in the `symbols-view` settings. You can see the full list of available icons by invoking the **Styleguide: Show** command and browsing the “Icons” section. The value can include the preceding `icon-` or can omit it; e.g., `icon-package` and `package` are both valid values. - -If this value is omitted, this provider will still attempt to match certain common tag values to icons. If `tag` is not present on the symbol, or is an uncommon value, there will be a blank space instead of an icon. +Nearly all the tags on [the aforementioned list](https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#symbolKind) will also apply an appropriate `icon` to their symbol when assigned. If you choose a tag name not on that list, or want to override the default, you can use the `symbol.icon` predicate described below. ###### symbol.tag @@ -181,3 +174,16 @@ The `symbol.tag` predicate will set the value of a symbol’s `tag` property to The `tag` property is used to supply a word that represents the symbol in some way. For conventional symbols, this will often be something like `class` or `function`. This provider will attempt to match certain common tag values to icons. This can be overridden by specifying an explicit `symbol.icon` value. + +###### symbol.icon + +```scm +(class_body (method_definition + name: (property_identifier) @name + (#set! symbol.icon "package") +)) +``` + +The icon to be shown alongside the symbol in a list. Will only be shown if the user has enabled the “Show Icons in Symbols View” option in the `symbols-view` settings. You can see the full list of available icons by invoking the **Styleguide: Show** command and browsing the “Icons” section. The value can include the preceding `icon-` or can omit it; e.g., `icon-package` and `package` are both valid values. + +If this value is omitted, this provider will still attempt to match certain common tag values to icons. If `tag` is not present on the symbol, or is an uncommon value, there will be a blank space instead of an icon. diff --git a/packages/symbol-provider-tree-sitter/lib/capture-organizer.js b/packages/symbol-provider-tree-sitter/lib/capture-organizer.js index 9086bb149..64e3f38e2 100644 --- a/packages/symbol-provider-tree-sitter/lib/capture-organizer.js +++ b/packages/symbol-provider-tree-sitter/lib/capture-organizer.js @@ -27,28 +27,66 @@ const PatternCache = { }, }; + +// Assign a default icon type for each tag — or what LSP calls “kind.” This +// list is copied directly from the LSP spec's exhaustive list of potential +// symbol kinds: +// +// https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#symbolKind function iconForTag(tag) { switch (tag) { - case 'function': - return 'icon-gear'; - case 'method': - return 'icon-gear'; - case 'namespace': - return 'icon-tag'; - case 'variable': - return 'icon-code'; - case 'class': - return 'icon-package'; - case 'constant': - return 'icon-primitive-square'; - case 'property': - return 'icon-primitive-dot'; - case 'interface': - return 'icon-key'; - case 'constructor': - return 'icon-tools'; + case 'file': + return 'icon-file'; case 'module': return 'icon-database'; + case 'namespace': + return 'icon-tag'; + case 'package': + return 'icon-package'; + case 'class': + return 'icon-puzzle'; + case 'method': + return 'icon-gear'; + case 'property': + return 'icon-primitive-dot'; + case 'field': + return 'icon-primitive-dot'; + case 'constructor': + return 'icon-tools'; + case 'enum': + return 'icon-list-unordered'; + case 'interface': + return 'icon-key'; + case 'function': + return 'icon-gear'; + case 'variable': + return 'icon-code'; + case 'constant': + return 'icon-primitive-square'; + case 'string': + return 'icon-quote'; + case 'number': + return 'icon-plus'; + case 'boolean': + return 'icon-question'; + case 'array': + return 'icon-list-ordered'; + case 'object': + return 'icon-file-code'; + case 'key': + return 'icon-key'; + case 'null': + return null; + case 'enum-member': + return 'icon-primitive-dot'; + case 'struct': + return 'icon-book'; + case 'event': + return 'icon-calendar'; + case 'operator': + return 'icon-plus'; + case 'type-parameter': + return null; default: return null; } @@ -66,9 +104,10 @@ class Container { this.capture = capture; this.node = capture.node; this.organizer = organizer; + this.props = capture.setProperties || {}; this.tag = capture.name.substring(capture.name.indexOf('.') + 1); - this.icon = iconForTag(this.tag); + this.icon = this.resolveIcon(); this.position = capture.node.range.start; } @@ -108,6 +147,13 @@ class Container { ); } + resolveIcon() { + let icon = this.props['symbol.icon'] ?? iconForTag(this.tag); + if (icon && !icon.startsWith('icon-')) + icon = `icon-${icon}`; + return icon; + } + toSymbol() { if (!this.nameCapture) return null; let nameSymbol = this.nameCapture.toSymbol(); @@ -115,7 +161,7 @@ class Container { name: nameSymbol.name, shortName: nameSymbol.shortName, tag: nameSymbol.tag ?? this.tag, - icon: nameSymbol.icon ?? iconForTag(nameSymbol.tag) ?? iconForTag(this.tag), + icon: nameSymbol.icon ?? this.icon, position: this.position }; diff --git a/packages/symbol-provider-tree-sitter/spec/symbol-provider-tree-sitter-spec.js b/packages/symbol-provider-tree-sitter/spec/symbol-provider-tree-sitter-spec.js index acc0228f7..ad8b7fdf3 100644 --- a/packages/symbol-provider-tree-sitter/spec/symbol-provider-tree-sitter-spec.js +++ b/packages/symbol-provider-tree-sitter/spec/symbol-provider-tree-sitter-spec.js @@ -248,6 +248,166 @@ describe('TreeSitterProvider', () => { grammar = editor.getGrammar(); }); + describe('symbol.context', () => { + beforeEach(async () => { + await grammar.setQueryForTest('tagsQuery', scm` + ( + (variable_declaration + (variable_declarator + name: (identifier) @name + value: [(arrow_function) (function)])) + (#set! symbol.context "something") + ) + `); + }); + + it('assigns a `context` property on each symbol', async () => { + let symbols = await getSymbols(editor, 'file'); + + expect(symbols[0].context).toBe('something'); + expect(symbols[0].position.row).toEqual(0); + + expect(symbols[1].context).toBe('something'); + expect(symbols[1].position.row).toEqual(1); + }); + }); + + describe('symbol.contextNode', () => { + beforeEach(async () => { + await grammar.setQueryForTest('tagsQuery', scm` + ( + (property_identifier) @name + (#eq? @name "push") + (#set! symbol.contextNode "parent.firstNamedChild") + ) + `); + }); + + it('assigns a `context` property on each symbol containing the text of the referenced node', async () => { + let symbols = await getSymbols(editor, 'file'); + + expect(symbols[0].name).toBe('push'); + expect(symbols[0].context).toBe('left'); + expect(symbols[0].position.row).toEqual(6); + + expect(symbols[1].name).toBe('push'); + expect(symbols[1].context).toBe('right'); + expect(symbols[1].position.row).toEqual(6); + }); + }); + + describe('symbol.icon', () => { + it('defines an `icon` property on each symbol', async () => { + await grammar.setQueryForTest('tagsQuery', scm` + ( + (variable_declaration + (variable_declarator + name: (identifier) @name + value: [(arrow_function) (function)])) + (#set! symbol.icon "book") + ) + + `); + + let symbols = await getSymbols(editor, 'file'); + console.log('symbols:', symbols); + + expect(symbols[0].icon).toBe('icon-book'); + expect(symbols[0].position.row).toEqual(0); + + expect(symbols[1].icon).toBe('icon-book'); + expect(symbols[1].position.row).toEqual(1); + }); + + it('supersedes an `icon` property assigned by a tag', async () => { + await grammar.setQueryForTest('tagsQuery', scm` + ( + (variable_declaration + (variable_declarator + name: (identifier) @name + value: [(arrow_function) (function)])) + (#set! symbol.tag "class") + (#set! symbol.icon "book") + ) + `); + + let symbols = await getSymbols(editor, 'file'); + + expect(symbols[0].icon).toBe('icon-book'); + expect(symbols[0].position.row).toEqual(0); + + expect(symbols[1].icon).toBe('icon-book'); + expect(symbols[1].position.row).toEqual(1); + }); + + it('supersedes an `icon` property inferred by its container', async () => { + await grammar.setQueryForTest('tagsQuery', scm` + ( + (variable_declaration + (variable_declarator + name: (identifier) @name + value: [(arrow_function) (function)])) + (#set! symbol.tag "class") + (#set! symbol.icon "book") + ) @definition.namespace + `); + + let symbols = await getSymbols(editor, 'file'); + + expect(symbols[0].icon).toBe('icon-book'); + expect(symbols[0].position.row).toEqual(0); + + expect(symbols[1].icon).toBe('icon-book'); + expect(symbols[1].position.row).toEqual(1); + }); + }); + + describe('symbol.tag', () => { + it('defines a `tag` property on each symbol', async () => { + await grammar.setQueryForTest('tagsQuery', scm` + ( + (variable_declaration + (variable_declarator + name: (identifier) @name + value: [(arrow_function) (function)])) + (#set! symbol.tag "class") + ) + `); + + let symbols = await getSymbols(editor, 'file'); + + expect(symbols[0].tag).toBe('class'); + expect(symbols[0].icon).toBe('icon-puzzle'); + expect(symbols[0].position.row).toEqual(0); + + expect(symbols[1].tag).toBe('class'); + expect(symbols[1].icon).toBe('icon-puzzle'); + expect(symbols[1].position.row).toEqual(1); + }); + + it('supersedes the `tag` property inferred by its container', async () => { + await grammar.setQueryForTest('tagsQuery', scm` + ( + (variable_declaration + (variable_declarator + name: (identifier) @name + value: [(arrow_function) (function)])) + (#set! symbol.tag "class") + ) @definition.namespace + `); + + let symbols = await getSymbols(editor, 'file'); + + expect(symbols[0].tag).toBe('class'); + expect(symbols[0].icon).toBe('icon-puzzle'); + expect(symbols[0].position.row).toEqual(0); + + expect(symbols[1].tag).toBe('class'); + expect(symbols[1].icon).toBe('icon-puzzle'); + expect(symbols[1].position.row).toEqual(1); + }); + }); + describe('symbol.strip', () => { beforeEach(async () => { await grammar.setQueryForTest('tagsQuery', scm` From db1a4c50a4ca8934c804fbe66c63b85083b72120 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Sun, 14 Jan 2024 13:41:18 -0800 Subject: [PATCH 5/9] [language-php] Update `tags.scm` --- packages/language-php/grammars/tree-sitter/queries/tags.scm | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/language-php/grammars/tree-sitter/queries/tags.scm b/packages/language-php/grammars/tree-sitter/queries/tags.scm index 3d03f5bc5..1326563d5 100644 --- a/packages/language-php/grammars/tree-sitter/queries/tags.scm +++ b/packages/language-php/grammars/tree-sitter/queries/tags.scm @@ -1,6 +1,6 @@ (namespace_definition - name: (namespace_name) @name) @module + name: (namespace_name) @name) @definition.namespace (interface_declaration name: (name) @name) @definition.interface @@ -11,7 +11,7 @@ (class_declaration name: (name) @name) @definition.class -(class_interface_clause [(name) (qualified_name)] @name) @impl +(class_interface_clause [(name) (qualified_name)] @name) @reference.interface (property_declaration (property_element (variable_name (name) @name))) @definition.field @@ -40,4 +40,4 @@ (member_call_expression name: (name) @name) @reference.call -(enum_declaration (name) @name) @definition.enum +((enum_declaration (name) @name) @definition.enum) From 8be56e2559187c115e13d35ef0dbfd504582f980 Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Tue, 16 Jan 2024 22:04:08 -0800 Subject: [PATCH 6/9] =?UTF-8?q?[symbols-view]=20Guard=20against=20error=20?= =?UTF-8?q?during=20teardown=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit …that can happen if the broker is destroyed before the providers. --- packages/symbols-view/lib/main.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/symbols-view/lib/main.js b/packages/symbols-view/lib/main.js index 43ae43648..4be87611a 100644 --- a/packages/symbols-view/lib/main.js +++ b/packages/symbols-view/lib/main.js @@ -101,9 +101,9 @@ module.exports = { return new Disposable(() => { if (Array.isArray(provider)) { - this.broker.remove(...provider); + this.broker?.remove(...provider); } else { - this.broker.remove(provider); + this.broker?.remove(provider); } }); }, From 67e41a1f11314ed94dbc3801822ac3346d20d88c Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Tue, 16 Jan 2024 22:05:52 -0800 Subject: [PATCH 7/9] [symbols-view] Infer icons from tag if icon is not present This should result in _lots_ more icons in the symbols list. --- packages/symbols-view/lib/symbols-view.js | 97 ++++++++++++++++++++--- 1 file changed, 85 insertions(+), 12 deletions(-) diff --git a/packages/symbols-view/lib/symbols-view.js b/packages/symbols-view/lib/symbols-view.js index 3c716b20d..0f5a8fb67 100644 --- a/packages/symbols-view/lib/symbols-view.js +++ b/packages/symbols-view/lib/symbols-view.js @@ -23,6 +23,70 @@ function validateListControllerProps(props) { )); } +// Assign a default icon type for each tag — or what LSP calls “kind.” This +// list is copied directly from the LSP spec's exhaustive list of potential +// symbol kinds: +// +// https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#symbolKind +function iconForTag(tag) { + switch (tag) { + case 'file': + return 'icon-file'; + case 'module': + return 'icon-database'; + case 'namespace': + return 'icon-tag'; + case 'package': + return 'icon-package'; + case 'class': + return 'icon-puzzle'; + case 'method': + return 'icon-gear'; + case 'property': + return 'icon-primitive-dot'; + case 'field': + return 'icon-primitive-dot'; + case 'constructor': + return 'icon-tools'; + case 'enum': + return 'icon-list-unordered'; + case 'interface': + return 'icon-key'; + case 'function': + return 'icon-gear'; + case 'variable': + return 'icon-code'; + case 'constant': + return 'icon-primitive-square'; + case 'string': + return 'icon-quote'; + case 'number': + return 'icon-plus'; + case 'boolean': + return 'icon-question'; + case 'array': + return 'icon-list-ordered'; + case 'object': + return 'icon-file-code'; + case 'key': + return 'icon-key'; + case 'null': + return null; + case 'enum-member': + return 'icon-primitive-dot'; + case 'struct': + return 'icon-book'; + case 'event': + return 'icon-calendar'; + case 'operator': + return 'icon-plus'; + case 'type-parameter': + return null; + default: + return null; + } +} + /** * A class for setting various UI properties on a symbol list palette. This is a * privilege given to the “main” (or _exclusive_) provider for a given task. @@ -318,24 +382,33 @@ class SymbolsView { return true; } + normalizeSymbol(symbol, provider) { + // We enforce these so that (a) we can show a human-readable name of the + // provider for each symbol (if the user opts into it), and (b) we can + // selectively clear cached results for certain providers without + // affecting others. + symbol.providerName ??= provider.name; + symbol.providerId ??= provider.packageName; + if (symbol.path) { + let parts = Path.parse(symbol.path); + symbol.directory = `${parts.dir}${Path.sep}`; + symbol.file = parts.base; + } + symbol.name = symbol.name.replace(/[\n\r\t]/, ' '); + + if (symbol.tag && !symbol.icon) { + symbol.icon = iconForTag(symbol.tag); + } + } + addSymbols(allSymbols, newSymbols, provider) { for (let symbol of newSymbols) { if (!this.isValidSymbol(symbol)) { console.warn('Invalid symbol:', symbol); continue; } - // We enforce these so that (a) we can show a human-readable name of the - // provider for each symbol (if the user opts into it), and (b) we can - // selectively clear cached results for certain providers without - // affecting others. - symbol.providerName ??= provider.name; - symbol.providerId ??= provider.packageName; - if (symbol.path) { - let parts = Path.parse(symbol.path); - symbol.directory = `${parts.dir}${Path.sep}`; - symbol.file = parts.base; - } - symbol.name = symbol.name.replace(/[\n\r\t]/, ' '); + + this.normalizeSymbol(symbol, provider); allSymbols.push(symbol); } } From 51f06683f61486eeda1b56043bd9022bc7e4f56a Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Mon, 29 Jan 2024 00:53:42 -0800 Subject: [PATCH 8/9] =?UTF-8?q?[symbols-view]=20Implement=20=E2=80=9Cprefi?= =?UTF-8?q?ll=20with=20selected=20text=E2=80=9D?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When enabled, will use the selected text in your editor as the search query in the symbols list. Prefers an empty string if the setting is disabled, or if the editor selection spans more than one buffer line. --- packages/symbols-view/lib/file-view.js | 31 +++---- packages/symbols-view/lib/main.js | 37 ++++++-- packages/symbols-view/lib/project-view.js | 23 ++--- packages/symbols-view/package.json | 33 ++++--- .../symbols-view/spec/symbols-view-spec.js | 90 +++++++++++++++++-- 5 files changed, 166 insertions(+), 48 deletions(-) diff --git a/packages/symbols-view/lib/file-view.js b/packages/symbols-view/lib/file-view.js index 94b362e6c..9df1823a6 100644 --- a/packages/symbols-view/lib/file-view.js +++ b/packages/symbols-view/lib/file-view.js @@ -7,7 +7,7 @@ const el = require('./element-builder'); const { badge, isIterable, timeout } = require('./util'); class FileView extends SymbolsView { - constructor (stack, broker) { + constructor(stack, broker) { super(stack, broker); this.cachedResults = new Map(); // Cached results can be partially invalidated. If a provider wants to @@ -87,12 +87,12 @@ class FileView extends SymbolsView { }); } - destroy () { + destroy() { this.editorsSubscription.dispose(); return super.destroy(); } - elementForItem ({ position, name, tag, icon, context, providerName }) { + elementForItem({ position, name, tag, icon, context, providerName }) { // Style matched characters in search results. const matches = match(name, this.selectListView.getFilterQuery()); @@ -137,12 +137,12 @@ class FileView extends SymbolsView { return el('li.two-lines', primary, secondary); } - didChangeSelection (item) { + didChangeSelection(item) { let quickJump = Config.get('quickJumpToFileSymbol'); if (quickJump && item) this.openTag(item); } - async didCancelSelection () { + async didCancelSelection() { this.abortController?.abort(); await this.cancel(); let editor = this.getEditor(); @@ -152,12 +152,12 @@ class FileView extends SymbolsView { this.initialState = null; } - didConfirmEmptySelection () { + didConfirmEmptySelection() { this.abortController?.abort(); super.didConfirmEmptySelection(); } - async toggle () { + async toggle(filterTerm = '') { if (this.panel.isVisible()) await this.cancel(); let editor = this.getEditor(); // Remember exactly where the editor is so that we can restore that state @@ -170,9 +170,10 @@ class FileView extends SymbolsView { let populated = this.populate(editor); if (!populated) return; this.attach(); + this.selectListView.update({ query: filterTerm }); } - serializeEditorState (editor) { + serializeEditorState(editor) { let editorElement = atom.views.getView(editor); let scrollTop = editorElement.getScrollTop(); @@ -182,32 +183,32 @@ class FileView extends SymbolsView { }; } - deserializeEditorState (editor, { bufferRanges, scrollTop }) { + deserializeEditorState(editor, { bufferRanges, scrollTop }) { let editorElement = atom.views.getView(editor); editor.setSelectedBufferRanges(bufferRanges); editorElement.setScrollTop(scrollTop); } - getEditor () { + getEditor() { return atom.workspace.getActiveTextEditor(); } - getPath () { + getPath() { return this.getEditor()?.getPath(); } - getScopeName () { + getScopeName() { return this.getEditor()?.getGrammar()?.scopeName; } - isValidSymbol (symbol) { + isValidSymbol(symbol) { if (!symbol.position || !(symbol.position instanceof Point)) return false; if (typeof symbol.name !== 'string') return false; return true; } - async populate (editor) { + async populate(editor) { let result = this.cachedResults.get(editor); let providersToQuery = this.providersWithInvalidatedCaches.get(editor); if (result && !providersToQuery?.size) { @@ -238,7 +239,7 @@ class FileView extends SymbolsView { } } - async generateSymbols (editor, existingSymbols = null, onlyProviders = null) { + async generateSymbols(editor, existingSymbols = null, onlyProviders = null) { this.abortController?.abort(); this.abortController = new AbortController(); diff --git a/packages/symbols-view/lib/main.js b/packages/symbols-view/lib/main.js index 4be87611a..0442bf8b9 100644 --- a/packages/symbols-view/lib/main.js +++ b/packages/symbols-view/lib/main.js @@ -1,4 +1,4 @@ -const { Disposable } = require('atom'); +const { Disposable, TextEditor } = require('atom'); const Config = require('./config'); const ProviderBroker = require('./provider-broker'); const Path = require('path'); @@ -32,9 +32,13 @@ module.exports = { this.workspaceSubscription = atom.commands.add( 'atom-workspace', { - 'symbols-view:toggle-project-symbols': () => { - if (!this.ensureProvidersExist()) return; - this.createProjectView().toggle(); + 'symbols-view:toggle-project-symbols': (event) => { + if (!this.ensureProvidersExist()) { + event.abortKeyBinding(); + return; + } + let text = this.getSelectedTextIfEnabled(event); + this.createProjectView().toggle(text); }, 'symbols-view:show-active-providers': () => { this.showActiveProviders(); @@ -45,12 +49,13 @@ module.exports = { this.editorSubscription = atom.commands.add( 'atom-text-editor:not([mini])', { - 'symbols-view:toggle-file-symbols': (e) => { + 'symbols-view:toggle-file-symbols': (event) => { if (!this.ensureProvidersExist()) { - e.abortKeyBinding(); + event.abortKeyBinding(); return; } - this.createFileView().toggle(); + let text = this.getSelectedTextIfEnabled(event); + this.createFileView().toggle(text); }, 'symbols-view:go-to-declaration': () => { if (!this.ensureProvidersExist()) return; @@ -66,6 +71,24 @@ module.exports = { migrateOldConfigIfNeeded(); }, + getSelectedTextIfEnabled(event) { + let editorView = event.target.closest('atom-text-editor'); + if (!editorView) return ''; + let editor = editorView.getModel(); + let selection = editor.getLastSelection(); + + // Don't use the selection if it spans more than one buffer line. + let range = selection.getBufferRange(); + if (range.start.row !== range.end.row) return ''; + + // Don't use the selection unless the associated config option is enabled. + let prefill = atom.config.get( + 'symbols-view.prefillSelectedText', + { scope: [editor.getGrammar()?.scopeName] } + ); + return prefill ? selection.getText() : ''; + }, + deactivate() { this.fileView?.destroy(); this.fileView = null; diff --git a/packages/symbols-view/lib/project-view.js b/packages/symbols-view/lib/project-view.js index 4dfe0d11f..449c1bf8e 100644 --- a/packages/symbols-view/lib/project-view.js +++ b/packages/symbols-view/lib/project-view.js @@ -4,7 +4,7 @@ const SymbolsView = require('./symbols-view'); const { isIterable, timeout } = require('./util'); module.exports = class ProjectView extends SymbolsView { - constructor (stack, broker) { + constructor(stack, broker) { // TODO: Do these defaults make sense? Should we allow a provider to // override them? super(stack, broker, { @@ -16,30 +16,31 @@ module.exports = class ProjectView extends SymbolsView { this.shouldReload = true; } - destroy () { + destroy() { return super.destroy(); } - toggle () { + toggle(filterTerm = '') { if (this.panel.isVisible()) { this.cancel(); } else { this.populate(); this.attach(); + this.selectListView.update({ query: filterTerm }); } } - didCancelSelection () { + didCancelSelection() { this.abortController?.abort(); super.didCancelSelection(); } - didConfirmEmptySelection () { + didConfirmEmptySelection() { this.abortController?.abort(); super.didConfirmEmptySelection(); } - isValidSymbol (symbol) { + isValidSymbol(symbol) { if (!super.isValidSymbol(symbol)) return false; if ( !(typeof symbol.file === 'string' && typeof symbol.directory === 'string') && @@ -50,22 +51,22 @@ module.exports = class ProjectView extends SymbolsView { return true; } - shouldUseCache () { + shouldUseCache() { let query = this.selectListView?.getQuery(); if (query && query.length > 0) return false; if (this.shouldReload) return false; return !!this.cachedSymbols; } - didChangeQuery () { + didChangeQuery() { this.populate({ retain: true }); } - clear () { + clear() { } - async populate ({ retain = false } = {}) { + async populate({ retain = false } = {}) { if (this.shouldUseCache()) { await this.updateView({ items: this.cachedSymbols }); return true; @@ -131,7 +132,7 @@ module.exports = class ProjectView extends SymbolsView { return true; } - async generateSymbols (editor, query = '', callback) { + async generateSymbols(editor, query = '', callback) { this.abortController?.abort(); this.abortController = new AbortController(); diff --git a/packages/symbols-view/package.json b/packages/symbols-view/package.json index fb1ec705a..8e1003300 100644 --- a/packages/symbols-view/package.json +++ b/packages/symbols-view/package.json @@ -13,39 +13,52 @@ "quickJumpToFileSymbol": { "default": true, "type": "boolean", + "order": 1, "description": "Automatically visit selected file-symbols as you navigate the symbols list." }, - "showProviderNamesInSymbolsView": { - "default": false, - "type": "boolean", - "description": "When enabled, the name of the provider will be shown alongside each result." - }, "showIconsInSymbolsView": { "default": true, "type": "boolean", + "order": 2, "description": "When enabled, an icon will be shown alongside a symbol if the symbol provider specifies one." }, + "useBadgeColors": { + "default": false, + "type": "boolean", + "order": 3, + "description": "Whether to use an assortment of colors for symbol badges. If enabled, each badge will be one of sixteen colors based on its text. Badge colors are generated automatically as hue variants of your theme’s ordinary badge color." + }, + "prefillSelectedText": { + "default": true, + "type": "boolean", + "order": 4, + "description": "When enabled, any text you may have selected in the active editor will be used as the search query in the symbols list." + }, + "showProviderNamesInSymbolsView": { + "default": false, + "type": "boolean", + "order": 5, + "description": "When enabled, the name of the provider will be shown alongside each result." + }, "preferCertainProviders": { "default": [], "type": "array", + "order": 6, "items": { "type": "string" }, "description": "A comma-separated list of preferred providers. Used to help break ties when more than one provider can contribute symbols. Anything on this list will be preferred over anything not on this list, and earlier items will be preferred over later items. (A provider can be identified by its official name or its package name; run the **Symbols View: Show Active Providers** command to see both values.)" }, - "useBadgeColors": { - "default": false, - "type": "boolean", - "description": "Whether to use an assortment of colors for symbol badges. If enabled, each badge will be one of sixteen colors based on its text. Badge colors are generated automatically as hue variants of your theme’s ordinary badge color." - }, "providerTimeout": { "default": 2000, "type": "number", + "order": 7, "description": "How long providers have to respond to symbol requests before this package gives up and shows the list. If a certain provider is particularly slow, you may have to increase this value. (Does not apply to project-wide symbol search **if** the list is already visible.)" }, "enableDebugLogging": { "default": false, "type": "boolean", + "order": 10, "description": "Whether to log certain diagnostic information to the console. (For example: which provider is chosen for a given task.)" } }, diff --git a/packages/symbols-view/spec/symbols-view-spec.js b/packages/symbols-view/spec/symbols-view-spec.js index e34fc7231..a788d2c89 100644 --- a/packages/symbols-view/spec/symbols-view-spec.js +++ b/packages/symbols-view/spec/symbols-view-spec.js @@ -79,7 +79,7 @@ function registerProvider(...args) { } describe('SymbolsView', () => { - let symbolsView, activationPromise, editor, directory, mainModule; + let symbolsView, activationPromise, editor, directory, mainModule, languageMode; beforeEach(async () => { jasmine.unspy(Date, 'now'); @@ -101,10 +101,9 @@ describe('SymbolsView', () => { atom.config.set('symbols-view.showIconsInSymbolsView', false); activationPromise = atom.packages.activatePackage('symbols-view'); - activationPromise.then(() => { + await activationPromise.then(() => { mainModule = atom.packages.getActivePackage('symbols-view').mainModule; }); - await activationPromise; await atom.packages.activatePackage('language-javascript'); jasmine.attachToDOM(getWorkspaceView()); }); @@ -117,8 +116,8 @@ describe('SymbolsView', () => { beforeEach(async () => { atom.config.set('symbols-view.providerTimeout', 500); await atom.workspace.open(directory.resolve('sample.js')); - let editor = atom.workspace.getActiveTextEditor(); - let languageMode = editor.getBuffer().getLanguageMode(); + editor = atom.workspace.getActiveTextEditor(); + languageMode = editor.getBuffer().getLanguageMode(); if (languageMode.ready) await languageMode.ready; }); @@ -144,6 +143,40 @@ describe('SymbolsView', () => { }); + it('prefills the query field if `prefillSelectedText` is `true`', async () => { + atom.config.set('symbols-view.prefillSelectedText', true); + registerProvider(DummyProvider); + await activationPromise; + spyOn(editor, 'getSelectedText').andReturn('Symbol on Row 13'); + await dispatchAndWaitForChoices('symbols-view:toggle-file-symbols'); + symbolsView = getSymbolsView(); + + expect(symbolsView.selectListView.refs.loadingMessage).toBeUndefined(); + expect(document.body.contains(symbolsView.element)).toBe(true); + expect(symbolsView.element.querySelectorAll('li').length).toBe(1); + + expect(symbolsView.element.querySelector('li:first-child .primary-line')).toHaveText('Symbol on Row 13'); + expect(symbolsView.element.querySelector('li:first-child .secondary-line')).toHaveText('Line 13'); + }); + + it('does not prefill the query field if `prefillSelectedText` is `false`', async () => { + atom.config.set('symbols-view.prefillSelectedText', false); + registerProvider(DummyProvider); + await activationPromise; + spyOn(editor, 'getSelectedText').andReturn('Symbol on Row 13'); + await dispatchAndWaitForChoices('symbols-view:toggle-file-symbols'); + symbolsView = getSymbolsView(); + + expect(symbolsView.selectListView.refs.loadingMessage).toBeUndefined(); + expect(document.body.contains(symbolsView.element)).toBe(true); + expect(symbolsView.element.querySelectorAll('li').length).toBe(5); + + expect(symbolsView.element.querySelector('li:first-child .primary-line')).toHaveText('Symbol on Row 1'); + expect(symbolsView.element.querySelector('li:first-child .secondary-line')).toHaveText('Line 1'); + expect(symbolsView.element.querySelector('li:last-child .primary-line')).toHaveText('Symbol on Row 13'); + expect(symbolsView.element.querySelector('li:last-child .secondary-line')).toHaveText('Line 13'); + }); + it('does not wait for providers that take too long', async () => { registerProvider(DummyProvider, VerySlowProvider); await activationPromise; @@ -614,6 +647,7 @@ describe('SymbolsView', () => { describe('when toggling project symbols', () => { beforeEach(async () => { await atom.workspace.open(directory.resolve('sample.js')); + editor = atom.workspace.getActiveTextEditor(); }); it('displays all symbols', async () => { @@ -636,6 +670,48 @@ describe('SymbolsView', () => { expect(symbolsView.element.querySelector('li:last-child .secondary-line')).toHaveText(`${relative}:13`); }); + it('prefills the query field if `prefillSelectedText` is `true`', async () => { + atom.config.set('symbols-view.prefillSelectedText', true); + registerProvider(DummyProvider); + await activationPromise; + spyOn(editor, 'getSelectedText').andReturn('Symbol on Row 13'); + await dispatchAndWaitForChoices('symbols-view:toggle-project-symbols'); + symbolsView = getSymbolsView(); + + expect(symbolsView.selectListView.refs.loadingMessage).toBeUndefined(); + expect(document.body.contains(symbolsView.element)).toBe(true); + expect(symbolsView.element.querySelectorAll('li').length).toBe(1); + + let root = atom.project.getPaths()[1]; + let resolved = directory.resolve('other-file.js'); + let relative = `${path.basename(root)}${resolved.replace(root, '')}`; + + expect(symbolsView.element.querySelector('li:first-child .primary-line')).toHaveText('Symbol on Row 13'); + expect(symbolsView.element.querySelector('li:first-child .secondary-line')).toHaveText(`${relative}:13`); + }); + + it('does not prefill the query field if `prefillSelectedText` is `false`', async () => { + atom.config.set('symbols-view.prefillSelectedText', false); + registerProvider(DummyProvider); + await activationPromise; + spyOn(editor, 'getSelectedText').andReturn('Symbol on Row 13'); + await dispatchAndWaitForChoices('symbols-view:toggle-project-symbols'); + symbolsView = atom.workspace.getModalPanels()[0].item; + + expect(symbolsView.selectListView.refs.loadingMessage).toBeUndefined(); + expect(document.body.contains(symbolsView.element)).toBe(true); + expect(symbolsView.element.querySelectorAll('li').length).toBe(5); + + let root = atom.project.getPaths()[1]; + let resolved = directory.resolve('other-file.js'); + let relative = `${path.basename(root)}${resolved.replace(root, '')}`; + + expect(symbolsView.element.querySelector('li:first-child .primary-line')).toHaveText('Symbol on Row 1'); + expect(symbolsView.element.querySelector('li:first-child .secondary-line')).toHaveText(`${relative}:1`); + expect(symbolsView.element.querySelector('li:last-child .primary-line')).toHaveText('Symbol on Row 13'); + expect(symbolsView.element.querySelector('li:last-child .secondary-line')).toHaveText(`${relative}:13`); + }); + it('asks for new symbols when the user starts typing', async () => { registerProvider(ProgressiveProjectProvider); spyOn(ProgressiveProjectProvider, 'getSymbols').andCallThrough(); @@ -756,6 +832,9 @@ describe('SymbolsView', () => { describe('when quickJumpToSymbol is true', () => { beforeEach(async () => { await atom.workspace.open(directory.resolve('sample.js')); + editor = atom.workspace.getActiveTextEditor(); + languageMode = editor.getBuffer().getLanguageMode(); + if (languageMode.ready) await languageMode.ready; }); it('jumps to the selected function', async () => { @@ -775,6 +854,7 @@ describe('SymbolsView', () => { // dev tools console? That seems to break it on a reliable basis. Not // sure why yet. it('restores previous editor state on cancel', async () => { + atom.config.set('symbols-view.prefillSelectedText', false); registerProvider(DummyProvider); await activationPromise; const bufferRanges = [{start: {row: 0, column: 0}, end: {row: 0, column: 3}}]; From 05de7dba2c2bed7eb1b2f7a898f58e636eb0758a Mon Sep 17 00:00:00 2001 From: Andrew Dupont Date: Mon, 29 Jan 2024 00:56:32 -0800 Subject: [PATCH 9/9] [symbols-view] Forgot this part. --- packages/symbols-view/lib/main.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/symbols-view/lib/main.js b/packages/symbols-view/lib/main.js index 0442bf8b9..a8aedeed7 100644 --- a/packages/symbols-view/lib/main.js +++ b/packages/symbols-view/lib/main.js @@ -86,7 +86,7 @@ module.exports = { 'symbols-view.prefillSelectedText', { scope: [editor.getGrammar()?.scopeName] } ); - return prefill ? selection.getText() : ''; + return prefill ? editor.getSelectedText() : ''; }, deactivate() {