Skip to content
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.

Sign up for GitHub

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

Improve outline #5536

Merged
merged 10 commits into from
Feb 3, 2023
+37 −5
Merged

Improve outline #5536

Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
e5d6b0b
Rework logic for name/details creation for document symbols
trolleyman Mar 9, 2022
0c301d8
Merge remote-tracking branch 'origin/master' into improve-outline-bre…
trolleyman Jan 15, 2023
d991996
Change logic
trolleyman Jan 16, 2023
7954c8d
Brace
trolleyman Jan 16, 2023
ae1091e
Only make changes for objects
trolleyman Jan 16, 2023
39fc7dd
Revert changes
trolleyman Jan 16, 2023
cf00aeb
Merge branch 'master' into improve-outline-breadcrumbs
trolleyman Jan 20, 2023
2cbebec
Simplify code
trolleyman Feb 3, 2023
072dd08
Merge branch 'improve-outline-breadcrumbs' of https://github.com/trol…
trolleyman Feb 3, 2023
a3d8056
Merge branch 'master' into improve-outline-breadcrumbs
dibarbet Feb 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
48 changes: 43 additions & 5 deletions src/features/documentSymbolProvider.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,13 @@ export default class OmnisharpDocumentSymbolProvider extends AbstractSupport imp
}
}

function createSymbols(elements: Structure.CodeElement[]): vscode.DocumentSymbol[] {
function createSymbols(elements: Structure.CodeElement[], parentElement?: Structure.CodeElement): vscode.DocumentSymbol[] {
let results: vscode.DocumentSymbol[] = [];

elements.forEach(element => {
let symbol = createSymbolForElement(element);
let symbol = createSymbolForElement(element, parentElement);
if (element.Children) {
symbol.children = createSymbols(element.Children);
symbol.children = createSymbols(element.Children, element);
}

results.push(symbol);
Expand All @@ -46,11 +46,49 @@ function createSymbols(elements: Structure.CodeElement[]): vscode.DocumentSymbol
return results;
}

function createSymbolForElement(element: Structure.CodeElement): vscode.DocumentSymbol {
function getNameAndDetailsForElement(element: Structure.CodeElement, parentElement?: Structure.CodeElement): { name: string, details: string } {
switch (element.Kind) {
case SymbolKinds.Class:
case SymbolKinds.Delegate:
case SymbolKinds.Enum:
case SymbolKinds.Interface:
case SymbolKinds.Struct:
case SymbolKinds.Namespace:
if (typeof parentElement === 'undefined') {
return { name: element.DisplayName, details: '' };
}
const prefix = `${parentElement.DisplayName}.`;
if (!element.DisplayName.startsWith(prefix)) {
return { name: element.DisplayName, details: '' };
}
const name = element.DisplayName.slice(prefix.length);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that in most cases the element.Name property should be the non-fully qualified name of the class/type/interace/etc.

e.g.
image

So I think we might not need to do any slicing here, instead just pass in the 'name' as the as the name and the displayName as the details

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review, nw about the delay :)

The problem is with namespace elements - without the slice the namespace Foo.Bar.Baz (without any parents) will resolve as Baz, which causes some confusion with the breadcrumbs, making it seem like the namespace is just Baz.

Copy link
Member

@dibarbet dibarbet Feb 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see what you mean, looks like the server just populates the name with the last identifier.

Can we do this:

  1. Limit the slicing to only happen for namespace case
  2. For the rest, use the name and display name as-is for name and details

We could change the server side to fix this, but seems like overkill for such a small change, so I'd rather not.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey - I've done that, with a slight tweak - I've used DisplayName for the name for some types, and Name for others, so that more information is given - for example, below each function's Name is just OverloadMethod, which is a bit worse IMO in the outliner on small sizes (and gives less information in the breadcrumb). Having it be the DisplayName is how it is currently. It's up to you though, if you think the Name version is better I can change it to that.

DisplayName

image

Name

image

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah absolutely, using DisplayName there looks better - good call.

if (name === element.DisplayName) {
return { name: element.DisplayName, details: '' };
}
return { name, details: element.DisplayName };

case SymbolKinds.Constant:
case SymbolKinds.Constructor:
case SymbolKinds.Destructor:
case SymbolKinds.EnumMember:
case SymbolKinds.Event:
case SymbolKinds.Field:
case SymbolKinds.Indexer:
case SymbolKinds.Method:
case SymbolKinds.Operator:
case SymbolKinds.Property:
case SymbolKinds.Unknown:
default:
return { name: element.DisplayName, details: '' };
}
}

function createSymbolForElement(element: Structure.CodeElement, parentElement?: Structure.CodeElement): vscode.DocumentSymbol {
const fullRange = element.Ranges[SymbolRangeNames.Full];
const nameRange = element.Ranges[SymbolRangeNames.Name];
const { name, details } = getNameAndDetailsForElement(element, parentElement);

return new vscode.DocumentSymbol(element.DisplayName, /*detail*/ "", toSymbolKind(element.Kind), toRange3(fullRange), toRange3(nameRange));
return new vscode.DocumentSymbol(name, details, toSymbolKind(element.Kind), toRange3(fullRange), toRange3(nameRange));
}

const kinds: { [kind: string]: vscode.SymbolKind; } = {};
Expand Down