-
Notifications
You must be signed in to change notification settings - Fork 35
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
Several tag related improvements #262
Conversation
@@ -639,8 +639,6 @@ module Completions = | |||
Some | |||
{ CompletionItem.Create(label) with | |||
Detail = Some detail | |||
// Use input as filter text to avoid any extra filtering on the editor's side |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@TheBlob42 did this line cause problems?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly. The FilterText
property is the string that the user input is matched against when filtering the completion candidates (see here). If you set this value for example to Some "foo"
and trigger the completion then typing "foo" in your editor should still show all completion candidates (this is an extreme example I know).
The problem here was that the filter text was set to the user input at the time of the completion candidates creation. Therefore every "additional" character would not match this text anymore and the completion candidates would be gone.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the fix here! I think there's a same-ish problem with other types of completions... I haven't fully figured out the deal with client side filtering of completion candidates.
Thanks for putting it together @TheBlob42! 💖 I'll have a close look shortly |
The one thing missing currently are dedicated tests for the DocumentSymbold and WorkspaceSymbol functionalities. If you're cool with the changes and the layout I would take some time to add those 🙂 |
Allow filtering of tags on the editor side to allow users to match their tag completion via text input.
Also include the symbol "prefix" (e.g. H1, H2, Tag) into the symbol name to allow better client side filtering.
- fix tags completion tests - add tags to document symbol tests - implement workspace symbol tests (including tags)
I have cleaned up my changes and the commit history. I have also added some missing tests (workspace symbols) and adapted the existing ones to incorporate the changes in relation to tags, so that they get tested as well. @artempyanykh please have a look. I am happy to discuss any changes being made 🙂 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Appreciate the improvement! 💖
I'll push a new release shortly |
Hi @TheBlob42 and @artempyanykh thanks for the change! Works great (feel free to close #261). |
@petobens good catch 🤔 would probably be worth its own issue |
Sure! Done. Thanks! |
I'm super excited this was added! I've been using tags for some time and I missed the LSP features for them. One enhancement I can suggest is supporting nested tags from Obsidian, like |
@Gelio please do create a separate issue for this. Would be great if you could elaborate more on what kind of support you want from nested tags, example use cases. |
Sure thing, I created #267 |
This PR improves tag related functionality with the following:
Obviously these changes are somewhat opinionated but I am open for discussion. Having tags available as symbols makes it easier to find related content. Especially for people using a Zettelkasten or other note taking system with marksman. Quickly finding all notes related to a certain tag or quickly jumping to a relevant section in a long markdown document should ease these workflows.
In regards to the filtering I always wondered why I could not enter any more text after the initial
#
to filter for a specific tag. I found the related comment in the code, but I don't understand why this decision was made.Problem to separate headings and tags
Since all symbols in markdown are of the type "String" (nothing else really fits) they are hard to distinguish from another. For the workspace symbols I have made the prefix (e.g. "Tag", "H1", "H2" etc.) filterable as well so that one could easily filter only for tags or heading. However this only makes sense for people using some sort of fuzzy searching instead of a literal string matching where this could lead to usability issues. I am open for other ideas on how to solve this
Once this is clarified I would also check the formatting and add tests for these new cases 🙂