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.

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

Allow debug adapters prioritize suggestions via breaking alphabetic sorting #78408

Closed
pavelfeldman opened this issue Aug 2, 2019 · 10 comments
Closed
Assignees
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Milestone

Comments

@pavelfeldman
Copy link
Member

Suggestions returned by the debug adapter are currently sorted by label then by type. This prevents debug adapters from suggesting their own completion order. For example, for JavaScript objects, useless __defineGetter__ suggestions are shown at the very top:

Screen Shot 2019-08-02 at 2 05 04 PM

Begin able to control the order allows adapters listing properties as they wish, for example by prototype, hence pushing the object's properties to the very bottom.

Screen Shot 2019-08-02 at 1 59 48 PM

@pavelfeldman
Copy link
Member Author

pavelfeldman commented Aug 2, 2019

I experimented with addressing it and the simplest way I found was prepending zero-width space to the suggestions in order to push them lower the stack. VS Code shows these suggestions, but does not quite like applying them out of the box. With a single line change to isWhitespaceAtPos that starts treating \u200B as a space makes it happy.

That might sound hacky, but formally it looks Ok and allows not introducing any additional completions settings and/or DAP capabilities. I'm happy to submit a PR if you find the approach reasonable.

@weinand weinand added debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality labels Aug 5, 2019
@weinand
Copy link
Contributor

weinand commented Aug 5, 2019

@isidorn We can take the "hacky" fix for now, but we should start addressing this in a better way, which might involve extending the DAP.

@isidorn
Copy link
Contributor

isidorn commented Aug 5, 2019

@pavelfeldman agree with your point that putting the useless properties at the end is the right thing to do.
@weinand as for extending the DAP I think the right apporach would be to go with sortText which we already expose in vscode.d.ts.

As for the hacky solution we can also go down that route, however for changes at isWhitespaceAtPos we need @jrieken approval since that would change all fuzzy scoring used by the suggest widget.
Thus I would lean towards extending the DAP.

@weinand
Copy link
Contributor

weinand commented Aug 5, 2019

Ok, I've created a DAP feature request for adding an optional sortText" to the "CompletionItem" type.

@jrieken
Copy link
Member

jrieken commented Aug 5, 2019

With a single line change to isWhitespaceAtPos that starts treating \u200B as a space makes it happy.

Generally that's a fair change - the list of whitespace characters in definitely not complete. Anyways, the proper way to tackle this is @weinand's sortText extension.

@weinand weinand added this to the August 2019 milestone Aug 5, 2019
@pavelfeldman
Copy link
Member Author

Any solution works for me. DAP sourText extension sounds great. It requires more changes in VS Code itself - it seemed fairly opinionated in how it wanted to alphabetically sort items, but overall sounds like a great addition!

@weinand
Copy link
Contributor

weinand commented Aug 20, 2019

@isidorn
I've added an optional "sortText" attribute to the "CompletionItem" type.
Please implement sorting completion items based on this.

@isidorn
Copy link
Contributor

isidorn commented Aug 20, 2019

Debug console now supports the sortText.
Try it out from Wednesday vscode insiders and let me know how it behaves.

@weinand
Copy link
Contributor

weinand commented Aug 20, 2019

I've added sample code for completions to mock-debug and verified that sorting completion items work.

@isidorn isidorn added verification-needed Verification of issue is requested verified Verification succeeded labels Aug 26, 2019
@isidorn
Copy link
Contributor

isidorn commented Aug 26, 2019

Adding verified label per @weinand comments. And seems to work for Pavel as well.

@vscodebot vscodebot bot locked and limited conversation to collaborators Oct 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debug Debug viewlet, configurations, breakpoints, adapter issues feature-request Request for new features or functionality verification-needed Verification of issue is requested verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

4 participants