symbols-view rolling fixes
#861
Conversation
|
I was using symbols view today and kept wanting for |
Ah, I was unaware of that setting in |
Oof. You know, I did go looking to see if there was a setting controlling this, but I totally overlooked that one. I'll move this out to an FR issue, since it doesn't feel relevant for a fix PR.
Oh, that would explain why it wasn't working for me! 😄 |
Specifying the icon via the `(#set! symbol.icon "foo")` predicate was having no effect.
…that can happen if the broker is destroyed before the providers.
This should result in _lots_ more icons in the symbols list.
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.
savetheclocktower
force-pushed
the
symbols-view-fixes
branch
from
1ef8ea8 to
51f0668
Compare
|
OK, I don't see myself adding any new stuff before 1.114, so this one's ready for review. |
|
I see you're interested in this landing for 1.114, but I feel at least as under-qualified to review this as I am for the tree-sitter stuff. If there's no one else to review it I can see what I can do between now and tomorrow evening in my timezone when Release day usually kicks off. |
|
I'll ask around in Discord about it. You've done more than your fair share. |
There was a problem hiding this comment.
This change looks great to me!
Overall it's full of fantastic stuff:
- Better iconography resolution (Although I hate that it has to be duplicated between
symbols-viewandsymbols-provider-tree-sitterbut understand that's the best way for now) - Better documentation in the type file, which is extremely helpful if we need to point any more community package maintainers there
- Brand new feature of auto querying your selected text, which I love
- And obviously loads more safety mechanisms, lots more nullish assignment, which I'm always a fan of.
The only thing really here that struck me as odd is the manual assignment of order in the configuration schema, not that this is a bad thing, I've just personally never seen it done, honestly made me do a double take lol, but no problem there.
Otherwise, I'm not the most qualified to review the new query file changes, but I'll happily trust you on those, since well your wrote them all originally.
Plus we have tests that seem to provide full coverage on everything here, so I'll happily say lets ship it!
Without it, your properties are sorted alphabetically. Less than ideal! |
| // symbol kinds: | ||
| // | ||
| // https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#symbolKind | ||
| function iconForTag(tag) { |
There was a problem hiding this comment.
I have the same need on my generic-lsp package, maybe we can think about movint this to the atom.ui namespace API?
There was a problem hiding this comment.
I wouldn't be opposed to that. Ideally, we figure out how to set a good default and allow the user to override it somehow.
Better to keep two different PRs here than try to shoehorn these fixes into the Tree-sitter rolling PR. This one should also land in time for 1.114.