-
Notifications
You must be signed in to change notification settings - Fork 114
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 CLI key --version #703
add CLI key --version #703
Conversation
ad339df
to
0207b1c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--version
is a nice addition. 👍
src/Nix/Options/Parser.hs
Outdated
<$> (fromMaybe Informational <$> optional | ||
(option | ||
(do | ||
a <- str | ||
if all isDigit a | ||
then pure $ decodeVerbosity (read a) | ||
else fail "Argument to -v/--verbose must be a number" | ||
) | ||
(short 'v' <> long "verbose" <> help "Verbose output") | ||
(option |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the loss of indentation makes the nesting less clear here, but, again, I don't care too much.
@Anton-Latukha It would be nice to include this in the next release. Do you want to make changes to this PR or can it be merged as-is? |
Oops, I had overlooked that you had marked the PR as a draft. I guess there's no need to wait for it then. |
There is no need to wait for it at all. It does not if PRs are green or a draft. Stable tImely releases are much more important than postponing the release to include current pipeline features into them (last one is the non-terminating algorithm). If some PRs need to get in - it is properly important to include fix PRs into the release. |
0207b1c
to
2320eff
Compare
Seems like this PR is affected by #706 too. |
2320eff
to
b64d84b
Compare
b64d84b
to
45fd8e0
Compare
45fd8e0
to
8bb47fc
Compare
Closes #702
The key itself is done in the first commit.
About
--version
option--version
option seems to not belong inhelp
head message (helper
). It, simply, is not a helper-related information.Tried to include
--version
innixOptions
but NOR was successful, NOR--version
fits to be innixOptions
abstraction, which holds interpretation keys.So, since it does not belong to
{helper, nixOptions}
-versionOpt
is separate abstraction.The proper way to get
version
number (in the current Haskell ecosystem) - is to take it from Cabal autogeneratedPaths_hnix
, so it is loaded right from thehnix.cabal version:
field, which is done here.The usage of Cabal provided metadata for this, makes direct
> ghci
command not viable (nor it was in HNix, the project can not be loaded from GHCi for other reasons). Nor GHCi expected to work, since there iscabal v2-repl
for loading packages.Proper
version
implementation goes against my request of having Nix package that is not tied to Cabal #545, but that target is not on the horizon, and we should decide on the hardcode policy before that, as project development would face tradeoffs, and some minor hardcode would be an answer some times.So I decided that returning a
--version
for people is beneficial, people now able to check and guard the version easily, and use the tool to automate things.The second commit is alignment clean-up.