-
Notifications
You must be signed in to change notification settings - Fork 697
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 "status" command #2882
Add "status" command #2882
Conversation
i should add an example for the output from
|
I've not looked at the detail of the patch, but in principle I think this is a good idea. I've been thinking of doing something like this for the nix-local-build branch where working out what the environment is is even more critical. |
UI bikeshedding: may be worth unifying this with |
@23Skidoo true; when i inserted "status" to the one more thing: the --all output (ellipses inserted by me):
|
UI Bikeshedding, I see the info command as separate, it's sort of the equivalent of the hackage package page for packages, rather than "what is going on with my current project". So I'm quite happy with them being separate commands. |
Yes, that's a valid point. On the other hand, the distinction between |
a0f5ece
to
8e9c7fa
Compare
How to i handle |
Doesn't importing |
@23Skidoo ah nice, i was not aware of that, thanks. |
I did some testing under windows now, looks fine. |
Are there further changes necessary before this can be merged, or is the main issue that somebody needs to find the time to review this? |
Disp.lineLength = 79, | ||
Disp.ribbonsPerLine = 1.0 | ||
} | ||
display = Disp.renderStyle defaultStyle . disp |
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.
Why is this change needed?
A changelog mention and some documentation would be nice. Documentation lives under |
multiDesc :: [String] -> String | ||
multiDesc l = "Multiple cabal files found.\n" | ||
++ "Please use only one of: " | ||
++ intercalate ", " l | ||
|
||
listPackageDescs :: FilePath -> IO [FilePath] |
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.
Please add a Haddock comment explaining what this function does.
Added some comments. The patches look self-contained (as they should be), so I think this PR can go in. |
[Just cabalInstallVersion, Just _cabalVersion] -> | ||
if cabalInstallVersion == Paths_cabal_install.version | ||
then return () | ||
else putStrLn $ "There is a different cabal (version " |
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.
Please change to "There is a different 'cabal' executable (version"
.
Just a general comment: I think the individual commits should probably be squashed to a single commit before merge...? |
Yes, that would be nice given that some of the intermediate states don't build cleanly and there is a spurious merge commit. |
Ok, i'll squash and rebase later. |
a26a852
to
d4d4dfa
Compare
I squashed all commits and rebased onto master.
Questions
|
I don't have a problem with it. I/someone can /cc you on PRs touching this module.
No, not
Yes, it needs to be reorganised. |
I added a paragraph about the For the changelog entry i moved it to 1.25 for now; if you want to still merge this into 1.24 this needs to be changed again. The testsuite for Cabal fails (locally) for some reason; but it does so on master in the same way, so I cannot test locally (https://gist.github.com/lspitzner/b9cb6a4831147767f24f). The AppVeyor failure seems to be unrelated to this commit as well (?) |
Configured compiler: | ||
ghc-7.10.3 | ||
~~~~~~~~~~~~~~~ | ||
|
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.
See here for the only addition apart from rebasing since last update.
The AppVeyor failure is unrelated to the PR. |
And sorry for the long wait. |
@lspitzner btw, would an optional JSON representation make sense for |
@hvr I cannot currently see a use-case for JSON output. My intention for this command is to provide some feedback to the human user; for automated settings I would probably tend towards full |
The "status" command prints a summary over several aspects of a cabal environment, such as the cabal and ghc versions, the package and its components, the package-databases, the sandbox etc.
# Conflicts: # Cabal/Distribution/Text.hs # Cabal/changelog # cabal-install/Distribution/Client/Freeze.hs # cabal-install/Distribution/Client/Setup.hs # cabal-install/Main.hs
Merged master again. Determining the currently configured compiler (version) seems to be broken now. Haven't investigated yet. |
I have no interest in working on this PR any longer. Has apparently bitrotten by now, although most of the conflicts look like easy to fix. I'll leave it to you to close this or leave it open; i don't know the policy for this. |
Closing in favour of #3643, which is an updated version of this PR. |
@lspitzner Thank you for working on this PR! I'll try to fix the remaining issues and get it merged. |
The "status" command prints a summary over several
aspects of a cabal environment, such as the cabal
and ghc versions, the package and its components,
the package-databases, the sandbox etc.
This is a first proposition for such a command; i am thankful for any feedback.
One thing that might be worth changing is behaviour with flags: Currently there are a few sections printed always, and a few that can be toggled with the flags of the command. Perhaps it would be better to:
a) provide a flag for each section
b) without any flags, print a default set (e.g. the current default)
c) with at least one flag, print exactly the sections selected
I have not tested with compiler != ghc or os != linux.