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

Fix Haskell ormolu fixer #4654

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

eahlberg
Copy link

@eahlberg eahlberg commented Nov 20, 2023

  1. Fix ormolu fixer doesn't work out of the box #4593 by enabling ormolu's --stdin-input-file option by default.
  2. Switch to the same design as other Haskell fixers (see Add better support for Haskell stack compiler tools #1851), which AFAIU is to improve support for stack. Note: I haven't been able to test this since I haven't found a way to make stack work on my setup (NixOS and cabal).

- Enable --stdin-input-file . option by default
- Use same pattern as other Haskell fixers
@eahlberg eahlberg changed the title Fix Haskell ormolu fixer WIP: Fix Haskell ormolu fixer Nov 20, 2023
@eahlberg eahlberg marked this pull request as draft November 20, 2023 14:58
@eahlberg eahlberg changed the title WIP: Fix Haskell ormolu fixer Fix Haskell ormolu fixer Nov 22, 2023
@eahlberg eahlberg marked this pull request as ready for review November 22, 2023 15:35
\ 'command': l:executable
\ . (empty(l:options) ? '' : ' ' . l:options)
\ . ' --stdin-input-file '
\ . ale#Escape(@%),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@% may not be the current file at the time the linter runs, as this code could be executed from a different buffer. Instead get the filename using the a:buffer variable. ALE has a shorthand for this where you can use %s in the command string. See :help ale-command-format-strings.

Have a look at changelogs and make sure the --stdin-input-file argument is available in old enough ormolu versions. If it's not available in older versions, you can make this fixer function still compatible with older versions with the ale#semver functions and a version check. You can grep the codebase for other fixers that run version checks.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for being slow to respond but thanks for the feedback!

@% may not be the current file at the time the linter runs, as this code could be executed from a different buffer. Instead get the filename using the a:buffer variable. ALE has a shorthand for this where you can use %s in the command string. See :help ale-command-format-strings.

👍

Have a look at changelogs and make sure the --stdin-input-file argument is available in old enough ormolu versions. If it's not available in older versions, you can make this fixer function still compatible with older versions with the ale#semver functions and a version check. You can grep the codebase for other fixers that run version checks.

ormolu is a Haskell program which uses Haskell PVP which AFAIU means the ale#semver functions won't work. I couldn't find usage of PVP version checks in the codebase, but I guess one option would be to create a similar ale#pvp module. Does that sound reasonable or do you know of a better way to solve this?

Copy link

stale bot commented Apr 22, 2024

This pull request has been automatically marked as stale because it has not been updated recently. Make sure to write tests and document your changes. See :help ale-dev for information on writing tests.
If your pull request is good to merge, bother w0rp or another maintainer again, and get them to merge it.

@stale stale bot added the stale PRs a bot will close automatically label Apr 22, 2024
@stale stale bot removed the stale PRs a bot will close automatically label May 1, 2024
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.

ormolu fixer doesn't work out of the box
2 participants