Haskell-language-server: Option to disable import lens plugin

Created on 24 Sep 2020  路  17Comments  路  Source: haskell/haskell-language-server

The import lens plugin is cool, however I believe it is not always useful and has some negative impact.

Why not always specify exact import lists?

I think specific import lists (or qualified imports) is great when importing from dependent libraries that have a lot of exports in a specific module.

However, when developing boring business applications with a large domain, you often want to break it down into a lot of very specific small types in small modules. This gives more type safety and also reduces recompilation time.

When you have a lot of modules with one thing in it, specifying import lists becomes burdensome and is essentially just repeating yourself. eg. import MyApp.Model.Person (Person(Person)).

What are the downsides of the ignoring the plugin?

It creates a lot of visual noise

image

It takes time to load

It takes about 3 seconds to load which moves the code down.


If this is a reasonable / desirable change I'd be happy to attempt it if I could get pointers on how to make it.

I think the other alternative which might work for me is a way to apply the plugin on all imports in one go. That might make it worth it to just always have specific import lists. Or both of these could be done.

imports plugin discussion enhancement

Most helpful comment

It would also be nice to disable it on a per import basis. I always use -XNoImplicitPrelude, then roll my own. Having all Prelude imports be explicit (things like Functor/Monad etc.) seems silly.

All 17 comments

I think a code action to apply all at once would be nice, it will solve the proble while adding a nice feature. But being able to disable it makes sense too.
Afaiu disable it will be easier though it would suppose:

  • create a new Boolean config field

https://github.com/haskell/haskell-language-server/blob/e390da54a90f2c6c02ce836a27f2d4c33a71f607/hls-plugin-api/src/Ide/Plugin/Config.hs#L44-L54

  • guard the code lens return result using that field

https://github.com/haskell/haskell-language-server/blob/e390da54a90f2c6c02ce836a27f2d4c33a71f607/plugins/default/src/Ide/Plugin/ImportLens.hs#L93-L102

  • you can get the config using LspFuncsas in

https://github.com/haskell/haskell-language-server/blob/ec827708f00e7e054525952f7de5c6ded046623d/hls-plugin-api/src/Ide/Plugin/Formatter.hs#L51

  • add the config field to vscode extension (as hlint)

https://github.com/haskell/vscode-haskell/blob/6e8e00c1cb3af98f34407fd7565e5430ebb909bd/package.json#L70-L76

I have a code action coming soon. But I don't plan to add any option to disable the code lens within the plugin, since I believe this is better solved by HLS allowing users to select which plugins they want to use.

Well, you may want to enable it per project or depending on what module are you editing, no?
Disable/enable all code lenses could be another option but maybe you want them for some use cases and not for others.

I recall a discussion on the hie issue tracker about carving our a namespace in the config, indexed by plugin ID, which would allow each individual plugin to provide config, in a composable and modular way.

See https://github.com/haskell/haskell-ide-engine/issues/800. Perhaps we should revisit that?

And the config should be managed by the client, e.g vscode has the ability to set global or workspace settings. Emacs too.

I was recently editing a file with lots of module imports, trying to edit the imports themselves, and having every other line be generated and with a lag made it really difficult. I would definitely like to disable this feature, but given its so prominent, I'd really like a code action to disable it rather than having to search around the docs, since its such an obvious thing to want. I definitely want all the other code lenses, so don't want to disable them for all.

I would really like to get this information in the hover information, as that's far less intrusive, and doesn't harm my editing lines experience.

I don't believe hover is a better UI for this, since the point is to see what every import statement is pulling at a glance.

The lag will be gone in the next release of HLS, which will include the change to use stale data.

The best UI would diff the import statement with the code lens, and show the diff instead of the original line.

VSCode allows to customise settings per project. A simple HLS setting to select which plugins are active would go a long way. There is no need to search around the docs, the HLS VS Code extension settings are very easy to find and users are already familiar with this.

I have a code action coming soon. But I don't plan to add any option to disable the code lens within the plugin, since I believe this is better solved by HLS allowing users to select which plugins they want to use.

Agree that it would be the right, generic solution.

I think it is useful, but not something you always want. Especially if you are not in the habit of explicitly listing the imports, which makes going anywhere near the import list a nightmare.

I think something like all of the following

  1. enable/disable via a configuration setting
  2. Present a lens once, at the top, to enable/disable the "full" version. It just says that there is potential work to be done.
    2.1 this has an action associated, that can flip it to the behaviour we see now
  3. Potentially make the activation if there is work to be done a code action, which could be limited to the specific import being considered.

This does assume a lot more interactivity with the code lens refresh than current clients do though. Perhaps something to investigate.

The best UI would diff the import statement with the code lens, and show the diff instead of the original line.

Rust Analyzer shows a lot of text in grey that "could" have been written by the user but wasn't. That works better when refactoring than the code lenses. As an example of what we could do:

image

There is no need to search around the docs, the HLS VS Code extension settings are very easy to find and users are already familiar with this.

I strongly doubt that is true. When I wanted to know how to disable a feature in the Rust Analyzer I raised a GitHub ticket: https://github.com/rust-analyzer/rust-analyzer/issues/6054. It's got a bunch of comments and thumbs up suggesting I wasn't the only one. There's also another near-identical bug someone else raised https://github.com/rust-analyzer/rust-analyzer/issues/6053. Given a feature that "gets in the way", I think it should be easy to turn off from nearby. If that adjusts the settings, that would be great, but I don't want to have to learn how we subqualify plugins (or even that HLS has plugins!) in order to reduce clutter.

That said, my preference would be to rework the feature to be less intrusive. One possibility:

  • A single lens to expand all imports in a block - so if you have 10 adjacent import lines, you could expand them all in one code lens. Still allows people to follow conventions about import explicitly from other packages, but everything from locally.
  • Use the grey text to give people information about what they import from where, in a way that doesn't interfere with editing (at least in VS Code).

Yeah, the grey text is exactly what I want. What magic is that?

I think my position is that HLS should empower both plugin writers and users. As a plugin writer I don't want to have to roll my own solution for toggling the plugin on/off. If the code lens for imports is too intrusive and we don't want to confuse new users, then have it disabled by default and let power users figure out how to enable it in the HLS settings page.

rust-analyzer seems to have a lot of custom code in its vscode extension: https://github.com/rust-analyzer/rust-analyzer/blob/master/editors/code/src/lsp_ext.ts

rust-analyzer seems to have a lot of custom code in its vscode extension

Yes, I am pretty sure the inline lens rendering is not in the lsp spec (yet).

I think we should strive to stay standard, and not put huge burdens on the vscode client developers

I also agree that a general mechanism to enable / disable plugins makes sense. That seems like a good focus for this particular issue. Although, I suspect the task is a bit too involved for me to develop having not contributed to HLS before.

Code action added in #436

It would also be nice to disable it on a per import basis. I always use -XNoImplicitPrelude, then roll my own. Having all Prelude imports be explicit (things like Functor/Monad etc.) seems silly.

It would also be nice to disable it on a per import basis. I always use -XNoImplicitPrelude, then roll my own. Having all Prelude imports be explicit (things like Functor/Monad etc.) seems silly.

Yup. Not only for Prelude, but also Prelude-esque imports. Like Reflex.Dom.Core.

I also agree that a general mechanism to enable / disable plugins makes sense.

For the above, it would be nice to have a more fire-grained configuration than that. So each plugin can (optionally) provide its own config type that the user can override. For the import lens plugin, taking the above into consideration, it would be something like:

data Config = Config 
  { ignoreImports :: [Import]
  } 

Which the user can ultimately configure in VSCode through Workspace or User settings JSON (or UI).

PRs accepted and encouraged!

Was this page helpful?
0 / 5 - 0 ratings