Merge pull request pulsar-edit#1133 from savetheclocktower/symbols-vi…

…ew-project-symbol-nonexclusivity [symbols-view] Allow project-wide symbol searches to consider multiple providers
ForestryAILab · Nov 20, 2024 · c6b4fdb · c6b4fdb
2 parents 39a4a4c + 54dd244
commit c6b4fdb
Show file tree

Hide file tree

Showing 4 changed files with 87 additions and 3 deletions.
diff --git a/packages/symbols-view/lib/project-view.js b/packages/symbols-view/lib/project-view.js
@@ -143,7 +143,11 @@ module.exports = class ProjectView extends SymbolsView {
     // longer need the symbols we asked for.
     let signal = this.abortController.signal;
 
-    let providers = await this.broker.select(meta);
+    // A user would probably expect this search to return symbols from all
+    // files in the project, regardless of their language. Instead of picking a
+    // “winning” provider as we usually do, we should instead consult _all_
+    // providers that consider themselves up to the task.
+    let providers = await this.broker.select(meta, { enforceExclusivity: false });
     if (providers?.length === 0) {
       console.warn('No providers found!');
       return null;

diff --git a/packages/symbols-view/lib/provider-broker.js b/packages/symbols-view/lib/provider-broker.js
@@ -175,7 +175,7 @@ module.exports = class ProviderBroker {
    * @returns {Promise<SymbolProvider[]>} A promise that resolves with a list
    *   of symbol providers.
    */
-  async select(meta) {
+  async select(meta, { enforceExclusivity = true } = {}) {
     let shouldLog = Config.get('enableDebugLogging');
     let exclusivesByScore = [];
     let results = [];
@@ -210,7 +210,14 @@ module.exports = class ProviderBroker {
       let { value: score } = outcome;
       let name = provider.name ?? 'unknown';
       let packageName = provider?.packageName ?? 'unknown';
-      let isExclusive = provider?.isExclusive ?? false;
+
+      // When `enforceExclusivity` is `false`, we'll treat all providers as
+      // non-exclusive, even the ones that indicate otherwise.
+      //
+      // It still falls on a provider to know whether to provide symbols or
+      // not. Any provider is free to inspect the metadata and return an empty
+      // set if it thinks it's inappropriate for its results to be considered.
+      let isExclusive = enforceExclusivity ? (provider?.isExclusive ?? false) : false;
 
       if (shouldLog)
         console.debug('Score for', provider.name, 'is:', score);

diff --git a/packages/symbols-view/spec/fixtures/providers/second-dummy-provider.js b/packages/symbols-view/spec/fixtures/providers/second-dummy-provider.js
@@ -0,0 +1,51 @@
+const { Point } = require('atom');
+
+function last(arr) {
+  return arr[arr.length - 1];
+}
+
+const ICONS = [
+  'icon-package',
+  'icon-key',
+  'icon-gear',
+  'icon-tag',
+  null
+];
+
+module.exports = {
+  packageName: 'symbol-provider-dummy-second',
+  name: 'Dummy (Second)',
+  isExclusive: true,
+  canProvideSymbols() {
+    return true;
+  },
+  getSymbols(meta) {
+    let { editor, type } = meta;
+    let results = [];
+    if (type === 'file') {
+      let count = editor.getLineCount();
+      // Put a symbol on every third line.
+      for (let i = 0; i < count; i += 3) {
+        results.push({
+          position: new Point(i, 0),
+          name: `(Second) Symbol on Row ${i + 1}`,
+          icon: ICONS[(i / 3) % (ICONS.length + 1)]
+        });
+      }
+    } else if (type === 'project') {
+      let root = last(atom.project.getPaths());
+      let count = editor.getLineCount();
+      // Put a symbol on every third line.
+      for (let i = 0; i < count; i += 3) {
+        results.push({
+          position: new Point(i, 0),
+          name: `(Second) Symbol on Row ${i + 1}`,
+          directory: root,
+          file: 'other-file.js',
+          icon: ICONS[i % (ICONS.length + 1)]
+        });
+      }
+    }
+    return results;
+  }
+};
diff --git a/packages/symbols-view/spec/symbols-view-spec.js b/packages/symbols-view/spec/symbols-view-spec.js
@@ -7,6 +7,7 @@ const SymbolsView = require('../lib/symbols-view');
 const { migrateOldConfigIfNeeded } = require('../lib/util');
 
 const DummyProvider = require('./fixtures/providers/dummy-provider');
+const SecondDummyProvider = require('./fixtures/providers/second-dummy-provider');
 const AsyncDummyProvider = require('./fixtures/providers/async-provider');
 const ProgressiveProjectProvider = require('./fixtures/providers/progressive-project-provider.js');
 const QuicksortProvider = require('./fixtures/providers/quicksort-provider.js');
@@ -696,6 +697,27 @@ describe('SymbolsView', () => {
       expect(symbolsView.element.querySelector('li:first-child .secondary-line')).toHaveText(`${relative}:13`);
     });
 
+    it('includes results from all providers, even if they claim to be exclusive', async () => {
+      registerProvider(DummyProvider);
+      registerProvider(SecondDummyProvider);
+
+      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(10);
+
+      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('(Second) Symbol on Row 13');
+      expect(symbolsView.element.querySelector('li:last-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);