Improve name export code action

[sergv]

Problem

I noticed that current code action that suggests to export otherwise unused toplevel name adds the name to module's export list in a very unsophisticated way which requires subsequent fix of style by hand, e.g.

module Mod 
  ( foo
  , bar
  ) where

becomes

module Mod 
  ( foo
  , bar, baz
  ) where

Solution

It occured to me that almost any style of the export list can be maintained if the code action will just look at the text between subsequest exported names, "\n , " in the example above, and just use that as separator between exported entries.

One thing to look for is that separators between names can be different, e.g. there can be haddoc comments like

module Mod 
  ( foo
  , bar
  -- * For testing
  , baz
  ) where

but this can be resolved by just looking at all names in the list, extracting all their separators and selecting the shortest one. If there's only one name exported then new name will be added with the separator used previously, module Mod (foo, bar) where.

[pepeiborra]

I noticed that current code action that suggests to export otherwise unused toplevel name adds the name to module's export list in a very unsophisticated way

This is a conscious choice - keep the logic simple, do as little guessing as possible, waste no time in formatting.

which requires subsequent fix of style by hand, e.g.

Why are you styling your code by hand, when the IDE can do it for you? It seems inconsistent to use the IDE to insert exports, but not use it to format your code.

[sergv]

Why are you styling your code by hand, when the IDE can do it for you

That's what I want - I want IDE to produce outputs that don't require any tweaks afterwards.

As to what you meant, you probably had in mind code formatters like brittany. Occasionally I use those but I'm yet to find one that I agree with 100% of the time. It's great that I have an option to apply one of those formatters via LSP, but I think I should not have to apply them to clean up after some code action.

It seems inconsistent to use the IDE to insert exports, but not use it to format your code.

Before hls I've been using dante (basically ghci) so I'm trying to gradually transition to hls for majority of my work. The hls has lots of plugins and capabilities which I'm yet to take advantage of.

On a different note, say I want to switch to ghc 9.2. I'll build hls with 9.2 to use it on my project, but some plugins may not be ported yet (e.g. brittany). Thus my favourite formatting plugin may be disabled, for a time, yet I'd like to use hls for checks, highlighting and code actions it still has.

Overall I think if a code action can maintain existing formatting at a reasonable cost (implementation complexity, runtime, etc), it's only beneficial to the user experience if it does so.

[Bodigrim]

There are plenty of people (me included), who do not use any code formatter. I work with a diverse set of open-source projects from various sources, which do not always have consistent formatting of imports even within the same file, and thus I resort to editting imports manually after HLS every time. This PR would me very valuable for my workflow.

[pepeiborra]

Why are you styling your code by hand, when the IDE can do it for you

That's what I want - I want IDE to produce outputs that don't require any tweaks afterwards.

As to what you meant, you probably had in mind code formatters like brittany. Occasionally I use those but I'm yet to find one that I agree with 100% of the time. It's great that I have an option to apply one of those formatters via LSP, but I think I should not have to apply them to clean up after some code action.

Automated code formatters are meant to be run on save.

On a different note, say I want to switch to ghc 9.2. I'll build hls with 9.2 to use it on my project, but some plugins may not be ported yet (e.g. brittany). Thus my favourite formatting plugin may be disabled, for a time, yet I'd like to use hls for checks, highlighting and code actions it still has.

We welcome contributions to upgrade plugins and/or dependencies to new GHC versions.

Overall I think if a code action can maintain existing formatting at a reasonable cost (implementation complexity, runtime, etc), it's only beneficial to the user experience if it does so.

I think that it's better to observe a separation of responsibilities, let the code formatters take care of style and keep the IDE out of that quagmire.

[pepeiborra]

There are plenty of people (me included), who do not use any code formatter. I work with a diverse set of open-source projects from various sources, which do not always have consistent formatting of imports even within the same file, and thus I resort to editting imports manually after HLS every time. This PR would me very valuable for my workflow.

Modern tooling is able to reformat only the lines that you edit

[pepeiborra]

Taking a step back, you had no way to know our design choices since they are not written anywhere. We should advertise our design preferences better so that future contributions are better aligned.

Thanks Sergey and hope to see more high quality PRs like this!

[sergv]

Thanks Pepe for getting through on it.

We welcome contributions to upgrade plugins and/or dependencies to new GHC versions.

I keep thinking about following case: what a user is supposed do in a situation when they want to use hls with GHC that their formatting plugin doesn't support yet? I'll take the brittany formatter as the one I'm most familiar with. Suppose the user already has a config for brittany and uses it everywhere. But to get hls with 9.2.2 support the brittany plugin has to be disabled. Should the user fix the plugin before carrying on their tasks? Plugin depends on the brittany package itself, so it has to be upgraded as well. Or should the user switch to another formatting plugin and come up with a config for it?

[pepeiborra]

It occured to me that almost any style of the export list can be maintained if the code action will just look at the text between subsequest exported names, "\n , " in the example above, and just use that as separator between exported entries.

Why stop there?

Thanks Pepe for getting through on it.

We welcome contributions to upgrade plugins and/or dependencies to new GHC versions.

I keep thinking about following case: what a user is supposed do in a situation when they want to use hls with GHC that their formatting plugin doesn't support yet? I'll take the brittany formatter as the one I'm most familiar with. Suppose the user already has a config for brittany and uses it everywhere. But to get hls with 9.2.2 support the brittany plugin has to be disabled. Should the user fix the plugin before carrying on their tasks? Plugin depends on the brittany package itself, so it has to be upgraded as well. Or should the user switch to another formatting plugin and come up with a config for it?

The user has several options:

1. Stick to a version of HLS with support for Brittany
2. Upgrade Brittany to support 9.2 (the HLS plugin is a thin wrapper that rarely needs changes)
3. Switch to another formatter with better support, like ormolu

[sergv]

Why stop there?

I certainly don't think that HLS should reimplement any code formatter. However if a reasonably simple and cheap heuristic is sufficient to provide good results and is useful for users then I think it's a candidate for inclusion.