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

Add option to respect default-extensions from .cabal files #759

Merged
merged 1 commit into from
Aug 12, 2021

Conversation

amesgen
Copy link
Member

@amesgen amesgen commented Aug 11, 2021

Closes #517
Closes #519

TODO:

  • Allow to explicitly set the path to the cabal file and to the source file (for input via stdin, for example for editor integrations)
  • Respect language (in advance of the GHC2021 language in GHC 9.2)
  • Test if this change significantly decreases performance. When ormolu is called with multiple files, we could cache the parsed .cabal files. EDIT see this comment
  • Add tests (could be further extended)
  • What do we want to do when the same file is part of different components? Right now, some arbitrary component is selected (via Map.union).
  • Make this new functionality discoverable, e.g. in the README.

This PR already revealed one issue, #758.

@amesgen
Copy link
Member Author

amesgen commented Aug 11, 2021

Concerning performance: This change is opt-in, so nothing will change in the default configuration. When -e is enabled, a small cost, dependent on the depth of the directory tree and the complexity of the .cabal file, has to be payed. When formatting ormolu, this means (using hyperfine and fd):

 $ hyperfine 'fd -e hs . src -X ormolu --mode check'
Benchmark #1: fd -e hs . src -X ormolu --mode check
  Time (mean ± σ):      1.148 s ±  0.031 s    [User: 1.119 s, System: 0.029 s]
  Range (min … max):    1.099 s …  1.198 s    10 runs

 $ hyperfine 'fd -e hs . src -X ormolu -e --mode check'
Benchmark #1: fd -e hs . src -X ormolu -e --mode check
  Time (mean ± σ):      1.291 s ±  0.032 s    [User: 1.250 s, System: 0.042 s]
  Range (min … max):    1.233 s …  1.323 s    10 runs

Considering that low hanging fruits like parallel formatting would make both cases much faster

 $ hyperfine 'fd -e hs . src -x ormolu --mode check'
Benchmark #1: fd -e hs . src -x ormolu --mode check
  Time (mean ± σ):     712.2 ms ±  40.7 ms    [User: 5.590 s, System: 0.981 s]
  Range (min … max):   662.1 ms … 790.8 ms    10 runs

 $ hyperfine 'fd -e hs . src -x ormolu -e --mode check'
Benchmark #1: fd -e hs . src -x ormolu -e --mode check
  Time (mean ± σ):     837.7 ms ±  20.6 ms    [User: 7.159 s, System: 1.104 s]
  Range (min … max):   810.9 ms … 872.6 ms    10 runs

I think that we don't have to worry too much about a performance regression in this PR.

@amesgen amesgen marked this pull request as ready for review August 11, 2021 15:14
app/Main.hs Outdated Show resolved Hide resolved
app/Main.hs Show resolved Hide resolved
src/Ormolu/Utils/Extensions.hs Show resolved Hide resolved
app/Main.hs Show resolved Hide resolved
default.nix Outdated Show resolved Hide resolved
src/Ormolu/Utils/Extensions.hs Outdated Show resolved Hide resolved
src/Ormolu/Utils/Extensions.hs Outdated Show resolved Hide resolved
src/Ormolu/Utils/Extensions.hs Outdated Show resolved Hide resolved
app/Main.hs Outdated Show resolved Hide resolved
@amesgen
Copy link
Member Author

amesgen commented Aug 12, 2021

Updated (01-10-2021): See vyorkin/ormolu.el#14 for a way to use this feature with ormolu.el.

This pull request was closed.
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.

Convenient default options Ask ghc/cabal for --ghc-opts
2 participants