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

Coherence of command-line formatters versus formatter plugins #411

Open
michaelpj opened this issue Sep 18, 2020 · 25 comments
Open

Coherence of command-line formatters versus formatter plugins #411

michaelpj opened this issue Sep 18, 2020 · 25 comments
Labels

Comments

@michaelpj
Copy link
Collaborator

As many people do, I have a gate in my CI that ensures that code passes a formatter before it is merged. That formatter (in this case stylish-haskell) is run as an executable.

If I also use HLS, I can format my code with stylish-haskell via the formatter plugin. But this will use the version of stylish-haskell that HLS was compiled with, not the one that I have (elsewhere) specified for use in CI.

This makes the in-editor formatting useless to me, as I will typically have to reformat with the executable version before my change will pass CI.

I can see two ways to keep things consistent.

  1. HLS follows environment

Ultimately I think this would mean that HLS would have to call the executable. I understand that this is undesirable since the current model allows us to use the nicer Haskell interfaces to tools and test them easily.

  1. Environment follows HLS

Perhaps we could have some way to format "as-if" using one of the HLS formatting plugins. For example:

  • Ship the formatter executables with HLS, so users can call those directly.
  • Expose the formatting capabilites through HLS itself as part of the CLI, e.g. haskell-language-server --format --formatter ormolu Foo.hs.
@michaelpj
Copy link
Collaborator Author

Oh, another irritation of HLS directly picking a version of the formatter is that then the "canonical" formatting of my codebase changes with the version of HLS. I can't pin stylish-haskell independently of HLS, as I would be able to if it called it as an external command.

@anka-213
Copy link
Contributor

I wonder if it would be possible to take this one step further and make more functionality available via the command line. For example, if I want to apply some fix everywhere in my code, it is often more convenient to automate from the command line than to try to coerce a GUI into doing it (or doing it everywhere manually). But maybe this is a completely separate issue.

@alanz
Copy link
Collaborator

alanz commented Sep 20, 2020

This could be ameliorated with the coming plugin architecture, which is specifically aimed at allowing a given user to configure exactly what plugins they want to use.

In time this should lead to a formatter publishing the appropriate plugin descriptor for use in HLS, then this becomes a simple matter of using the same version in both places.

@michaelpj
Copy link
Collaborator Author

How would the plugin API help here? Would the formatter provider have to provide N different plugins for N different versions? Or would they be loaded dynamically somehow so the user can choose which one? It seems difficult to make this work in the model where HLS statically depends on a particular formatter (at a particular version).

@alanz
Copy link
Collaborator

alanz commented Sep 21, 2020

Given the plugin descriptor is just a set of callbacks, depending on the separately provided hls-plugin-api, and specified in a thin Main.hs (

idePlugins :: Bool -> IdePlugins
idePlugins includeExamples = pluginDescToIdePlugins allPlugins
where
allPlugins = if includeExamples
then basePlugins ++ examplePlugins
else basePlugins
basePlugins =
[ GhcIde.descriptor "ghcide"
, Pragmas.descriptor "pragmas"
, Floskell.descriptor "floskell"
, Fourmolu.descriptor "fourmolu"
, Tactic.descriptor "tactic"
-- , genericDescriptor "generic"
-- , ghcmodDescriptor "ghcmod"
, Ormolu.descriptor "ormolu"
, StylishHaskell.descriptor "stylish-haskell"
, Retrie.descriptor "retrie"
#if AGPL
, Brittany.descriptor "brittany"
#endif
, Eval.descriptor "eval"
, ImportLens.descriptor "importLens"
]
examplePlugins =
[Example.descriptor "eg"
,Example2.descriptor "eg2"
]
), it should be perfectly feasible for a given formatter to provide it's own instance of the required descriptor. The only requirement is that it is compatible with the hls-plugin-api used in the underlying hls, and then allows you to precisely specify which version you want.

@michaelpj
Copy link
Collaborator Author

Okay, but I'd still need to recompile haskell-language-server using this plugin, wouldn't I? If I'm willing to do that I can just put constraint: stylish-haskell==whatever to get the version I want.

@michaelpj
Copy link
Collaborator Author

I also note that any solution which involves re-compiling a Haskell program rather defeats our attempt to distribute static binaries!

@alanz
Copy link
Collaborator

alanz commented Sep 29, 2020

I think we will end up shipping the binary for the "all included" version (and maybe the ghcide-only version), and leave it up to each site to build their own versions with custom plugin sets.

It's more about providing the capability.

@michaelpj
Copy link
Collaborator Author

See also cachix/git-hooks.nix#55

@michaelpj
Copy link
Collaborator Author

I was thinking about this again today. I wonder if a compromise solution would be to provide more options. I can think of two ways to do that:

  • A global option formattersUseExectuables, which would tell formatting plugins to shell out to executables instead of calling the library directly.
  • Additional formatting provider options that call executables, so we might have both stylish-haskell and stylish-haskell (executable).

@Profpatsch
Copy link

then this becomes a simple matter of using the same version in both places.

Easy said, but I don’t think this is really a simple matter. For example, in order for the CI to use exactly the same formatter, it would have to do the following steps:

  1. set up HLS
  2. ask HLS for the formatter version
  3. set up the formatter (which might work completely different for every formatter, if at all possible, and might take a lot of time)
  4. run the formatter

This does not align with e.g. my goal of having a linting phase <1 minute.


what if, instead, I could do:

  1. set up HLS
  2. run HLS in a formatting mode and tell it to reformat all source code files

This would solve the mismatch problem.

@Profpatsch
Copy link

It's more about providing the capability.

I politely disagree.

Formatting is a fundamental part of writing good, modern Haskell code, and having a formatting check in CI is a very important requirement for any non-hobbyist codebase.

Thus, saying “we just provide the capability, but it is up to the user to find out about it and actually figure out how to use that capability” is not really helpful for setting best practices for Haskell developers.

What I’d like to see is HLS take a bold step towards encouraging best practices, which includes making CI integration easy to set up.

@michaelpj
Copy link
Collaborator Author

Coming back to this, there's already a bit of abstraction over formatting plugins in hls-plugin-utils, and I think it would not be too hard to extend it so that all formatting plugins provided a generic CLI command that can format a file or the project.

@georgefst
Copy link
Collaborator

Coming back to this, there's already a bit of abstraction over formatting plugins in hls-plugin-utils, and I think it would not be too hard to extend it so that all formatting plugins provided a generic CLI command that can format a file or the project.

I'd love to see this. I've actually been thinking about this problem for a while, and I'm glad to see I'm not the only one. I'd been meaning to add a "use CLI" option to the Fourmolu plugin, but something more general would obviously be even better.

It would help greatly in situations like that described in #2649 (comment), where I want to be able to use the latest version of Fourmolu on a GHC-8.10 codebase, but can't because of dependency conflicts out of Fourmolu's control.

@georgefst
Copy link
Collaborator

I'd been meaning to add a "use CLI" option to the Fourmolu plugin, but something more general would obviously be even better.

#2763.

To be clear this implement's OP's option 1, for Fourmolu only for now.

@michaelpj
Copy link
Collaborator Author

I'd love to see this. I've actually been thinking about this problem for a while, and I'm glad to see I'm not the only one. I'd been meaning to add a "use CLI" option to the Fourmolu plugin, but something more general would obviously be even better.

I actually meant the reverse in that comment! I meant something like: if we have a formatting plugin, then the plugin could provide a subcommand to HLS so that you can do:

haskell-language-server format stylish-haskell Foo.hs

And it would run the formatter "as if" you had called format-buffer in the editor.

That way you would be sure they line up wrt configuration options, passing in extension flags from HLS, etc.

But I also like your approach!

@georgefst
Copy link
Collaborator

I actually meant the reverse in that comment!

Ah, my bad for not reading thoroughly. I assumed you were thinking the same thing as me since I saw "HLS would have to call the executable", but I see we're not talking about the same executable.

@michaelpj
Copy link
Collaborator Author

Note that the argument about passing the right options to tools applies to other things, e.g. it might be convenient to have haskell-language-server hlint, because sometimes HLS can do a better job than standalone hlint of working out what the enabled language extensions are.

@sir4ur0n
Copy link
Collaborator

sir4ur0n commented Jan 9, 2023

I found #2763 which apparently added a configuration flag to use the CLI fourmolu rather than the built-in one, but I could not find any similar option for other tools, e.g. ormolu.

Does someone know if this is possible please? 🙏

@georgefst
Copy link
Collaborator

Does someone know if this is possible please? 🙏

I don't believe so. It should be pretty straightforward to adapt the Fourmolu plugin's CLI code for Ormolu.

@fendor
Copy link
Collaborator

fendor commented Jan 9, 2023

Other to hls-cabal-fmt-plugin and hls-fourmolu-plugin, no other formatter uses the cli version of the underlying tool, afaik.

@chris-martin
Copy link

Each project has its own separate requred formatting tool (which is usually provided in the project's nix shell). Formatting is a matter of conformity to a project, not of personal preference, so my editor's "default" formatter is totally irrelevant. These rules were often put in place before HLS existed, and may be more specific than HLS is able to fully support - e.g. I may need not just fourmolu, but specifically fourmolu version 0.12.

The "environment follows HLS" option might be useful for some who are adopting new style rules or willing to change them, especially if HLS can do anything that other tools can't do, but it would be useless for most of my work. I like HLS being an optional tool and I don't want to force my colleagues to use it, nor do I want to run it in CI. It's hard to imagine, for comparison, a Java developer who runs IntelliJ in a github action to do style checks?

I think there really needs to be invented some standard way (I know, I know) for a repo to specify its formatter, including an option to just point to an executable, that an editor plugin will simply run. However, I do not think that editor plugin should be HLS, because HLS takes so long to load. I am perpetually getting "there is no formtter installed" popups in VSCode when I try to format a file that HLS is still compiling. This should not be necessary; formatters can run without the slow heavyweight analysis that loading into HLS requires.

@michaelpj
Copy link
Collaborator Author

I am perpetually getting "there is no formtter installed" popups in VSCode when I try to format a file that HLS is still compiling

That sounds like a separate bug, can you open a ticket? I'm surprised that that happens, since I'd have thought we only need the parsed module.

sir4ur0n pushed a commit to sir4ur0n/haskell-language-server that referenced this issue Aug 18, 2023
sir4ur0n pushed a commit to sir4ur0n/haskell-language-server that referenced this issue Aug 21, 2023
sir4ur0n pushed a commit to sir4ur0n/haskell-language-server that referenced this issue Aug 22, 2023
sir4ur0n pushed a commit to sir4ur0n/haskell-language-server that referenced this issue Aug 31, 2023
@michaelpj
Copy link
Collaborator Author

#3788 is possibly an example of where HLS can do a "better" job of formatting with fourmolu than the exact same CLI version because it knows more fixities. Of course, in this case having "better" formatting is annoying because they don't line up.

This makes me still want haskell-language-server format...

@sir4ur0n
Copy link
Collaborator

sir4ur0n commented Sep 6, 2023

About the original question: IMHO no matter what, HLS must provide a way to use external binaries, so option 1.

  1. Some people don't use an LSP but still use a formatter
  2. Some people use a different LSP than HLS
  3. Some people use HLS but cannot choose the version of a formatter to use
    E.g. "Company policy is to use formatter X in version Y.Z".

As a rule of thumb, an editor, LSP or really any tool should never be invasive: it must always elegantly coexist with people who don't use it. Otherwise HLS would be closing the ecosystem, preventing non-HLS users to collaborate with HLS users on the same code base.

Option 2 can be provided additionally on top of option 1, though in my opinion it is not a good idea:

  • it requires double maintenance (slows down the dev process of HLS)
  • it increases complexity
  • it is harder for users to understand how to use HLS

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

No branches or pull requests

9 participants