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

'Create Table of Contents' code action should not be suggested always #96

Closed
3 tasks
artempyanykh opened this issue Oct 2, 2022 · 7 comments
Closed
3 tasks
Labels
bad UX good first issue Good for newcomers

Comments

@artempyanykh
Copy link
Owner

artempyanykh commented Oct 2, 2022

  • Change the code to suggest 'Create ToC' when either
    • the requested range overlaps with the 1st line of the document,
    • code action kind is 'source'.
  • 'Update ToC' should be suggested anywhere in the document still (as is now).
  • Update the docs
@rchl
Copy link

rchl commented Oct 2, 2022

What does suggest 'Create ToC' only on the 1st line of the document mean exactly?
That the code action should only be returned if the asked for range includes first line?
If that's the case then you should also ensure that the code action is returned regardless of the asking range if explicitly requesting source code actions. Otherwise this change would cause issues for the Source actions context menu item.

@artempyanykh artempyanykh changed the title 'Create Table of Contents' code action should be suggested only on the 1st line of the document 'Create Table of Contents' code action should not be suggested always Oct 2, 2022
@artempyanykh
Copy link
Owner Author

Thanks for mentioning source code action kind @rchl! I've updated the todos to reflect the condition more precisely, e.g. either source or the range overlapping with 1st line of the document.

@David-Else
Copy link

Is it possible that Marsksman could not suggest creating a TOC at all, but instead it is available as an LSP command? If the document had a TOC it could still provide a code action to update it. I think this would be the best solution as most documents don't have a TOC and an LSP command seems a more natural solution?

Helix is about to get commands added, and Neovim has them already, I don't know the status in other editors, but it is official LSP spec so it should be available.

@rchl
Copy link

rchl commented Oct 28, 2022

I think that a code action of source type is appropriate for this use case.

Custom commands typically require a bit of glue code on the client side so I'm not sure what would be the benefit of that over source code actions.

@David-Else
Copy link

@rchl Are you saying that custom commands can't just be called via the client, they need specific code on the client side? If so, that would be bad news.

@rchl
Copy link

rchl commented Oct 28, 2022

LSP servers can only tell clients the command IDs they support (for example marksman.create_toc). The client neither has a nice title for that command nor a schema to know what arguments to pass (in case command takes arguments) so typically those missing bits need to be added manually to the client configuration.

Clients could expose command IDs and let users trigger them but that wouldn't be very useful in most cases as many commands require arguments.

@artempyanykh
Copy link
Owner Author

Marksman now supports user- and project-level config. With #109 users who don't care about ToC can disable it globally in their user-config and then re-enable on a project-by-project basis if needed.

By default, ToC is enabled but this is something to potentially reconsider in future.

Re: using code action's range and context to decide whether to suggest the ToC code action or not -- the problem here is that different editors, different LSP clients and different plugins all behave differently and have their own UI/UX for code actions. I hope that the config option will address 90% of the UX problems with ToC code action (particularly for non-VSCode/non-ST users where LSP clients don't do any filtering of code actions).

Closing this issue. It's OK to reopen it though, if somebody feels strongly about it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bad UX good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants