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

Several tag related improvements #262

Merged
merged 4 commits into from
Nov 20, 2023
Merged

Conversation

TheBlob42
Copy link
Contributor

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 🙂

@@ -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
Copy link
Owner

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?

Copy link
Contributor Author

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.

Copy link
Owner

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.

@artempyanykh
Copy link
Owner

Thanks for putting it together @TheBlob42! 💖 I'll have a close look shortly

@TheBlob42
Copy link
Contributor Author

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)
@TheBlob42
Copy link
Contributor Author

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 🙂

@TheBlob42 TheBlob42 changed the title Draft: Several tag related improvements Several tag related improvements Nov 19, 2023
Copy link
Owner

@artempyanykh artempyanykh left a 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! 💖

@artempyanykh artempyanykh merged commit dd5b4f5 into artempyanykh:main Nov 20, 2023
3 checks passed
@artempyanykh
Copy link
Owner

I'll push a new release shortly

@petobens
Copy link

Hi @TheBlob42 and @artempyanykh thanks for the change! Works great (feel free to close #261).
One minior comment: if I have an url like https://docs.google.com/spreadsheets/d/1Rr7GcfjUwhZfylUk0FGL0Et4kYF5etl49OpRwRmhhlg/edit#gid=1116260529&range=D12 marksman will treat this as tag. Is it possible to filter out these cases?

@TheBlob42
Copy link
Contributor Author

@petobens good catch 🤔 would probably be worth its own issue

@petobens
Copy link

Sure! Done. Thanks!

@Gelio
Copy link

Gelio commented Nov 21, 2023

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 #home/chore, #work/meeting. This would make my Neovim authoring experience even better. I can create an issue for it if you'd like

@artempyanykh
Copy link
Owner

@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.

@Gelio
Copy link

Gelio commented Nov 22, 2023

Sure thing, I created #267

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants