Fix #1578 - Redesign option parsing for main executable #1671
Conversation
|
I will investigate the build failures and mark it as ready to be reviewed after making it build on all envs. |
|
@fendor Could you also find a moment to do a review more focused on the style of the code and best practices? I'm willing to learn in the process and to do changes meant to make the new code consistent with the rest of the codebase. |
gdziadkiewicz
force-pushed
the
feature/#1578_-_Redesign_option_parsing_for_main_executable
branch
from
dfb732b to
ff6db7b
Compare
|
As a minimum, please update the description so that when it lands we know what it is about, without having to refer to a (possibly no longer existent) github repo. |
gdziadkiewicz
force-pushed
the
feature/#1578_-_Redesign_option_parsing_for_main_executable
branch
from
ff6db7b to
369ccfe
Compare
|
@alanz I changed the commit message. Please check if it is acceptable now |
gdziadkiewicz
changed the title
gdziadkiewicz
force-pushed
the
feature/#1578_-_Redesign_option_parsing_for_main_executable
branch
from
369ccfe to
a77935f
Compare
|
The CI fails don't seem to be connected to this change. |
gdziadkiewicz
force-pushed
the
feature/#1578_-_Redesign_option_parsing_for_main_executable
branch
from
a77935f to
951b2be
Compare
There was a problem hiding this comment.
Awesome! I tried my best to be nitpicky, but I just have nothing to criticise except for line length (and even that is debatable and personal preference)! Your code was easy to read and easy to follow!
I can say nothing more than, great job!
I hope you decide, and can find the time, to contribute more!
| <$> (ProjectLoadingOpts | ||
| <$> flag False True | ||
| ( long "dry-run" | ||
| <> help "Perform a dry-run of loading files. Only searches for Haskell source files to load. Does nothing if run as LSP server." |
There was a problem hiding this comment.
Line too long, limiting yourself to 80 chars per line makes it easier to read on small monitors
|
|
||
| spec :: Spec | ||
| spec = do | ||
| let defaultGlobalOptions = GlobalOpts False Nothing Nothing False Nothing False (ProjectLoadingMode $ ProjectLoadingOpts False []) |
There was a problem hiding this comment.
Line a bit too long, as well.
|
We will merge next week, when some time has passed since the release |
|
We will merge next week, when some time has passed since the last release |
gdziadkiewicz
deleted the
feature/#1578_-_Redesign_option_parsing_for_main_executable
branch
What has been done?
I extracted mode-specific
GlobalOptsfields into separate types, updated the parsing logic to handle new types as an alternative and updated the places whereGlobalOptswere used.How did I test it?
Added unit tests for option parser exposed by
Haskell.Ide.Engine.Options.