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

Systematic cleanup and colorization of stack output #2650

Closed
8 tasks
mgsloan opened this issue Sep 28, 2016 · 9 comments
Closed
8 tasks

Systematic cleanup and colorization of stack output #2650

mgsloan opened this issue Sep 28, 2016 · 9 comments

Comments

@mgsloan
Copy link
Contributor

mgsloan commented Sep 28, 2016

This issue is tracking doing a systematic sweep of the stack code and converting it to use the pretty-printer where appropriate.

I added some pretty print machinery to stack. In particular, we now have the following TH functions:

  • $prettyDebug is just like $logDebug, except it expects an AnsiDoc value.
  • $prettyInfo is just like $logInfo, except it expects an AnsiDoc value.
  • $prettyWarn is similar to $logWarn, but it prepends a newline and "Warning: " colored yellow.
  • $prettyError is similar to $logError, but it prepends a newline and "Error: " colored red.

One thing to note is that I would prefer colorization to be consistent across all of the stack messages. So, for the most part, colors should not be directly used in the stack source. Instead, add type and context specific utilities to `Stack.PrettyPrint. For example, I have these ones for plan construction:

displayTargetPkgId :: PackageIdentifier -> AnsiDoc
displayTargetPkgId = cyan . display

displayCurrentPkgId :: PackageIdentifier -> AnsiDoc
displayCurrentPkgId = yellow . display

displayErrorPkgId :: PackageIdentifier -> AnsiDoc
displayErrorPkgId = errorRed . display

Related issues that this work would ideally cover:

@mgsloan
Copy link
Contributor Author

mgsloan commented Sep 28, 2016

Assigning myself, but if someone is interested in learning this pretty print stuff and doing the work, feel free to take it over / pitch in!

Are you interested @luigy ? I know you were interested in improving some aspects of stack's CLI display.

@kadoban
Copy link
Collaborator

kadoban commented Aug 14, 2017

I think I'd be interested enough to take on at least some of this. Is there a branch or any context I should look at, or just start in s/S/PrettyPrint.hs and look for places it should be used and etc.?

@mgsloan
Copy link
Contributor Author

mgsloan commented Aug 14, 2017

@kadoban Awesome! I think the best place to start would be uses of logWarn, since there are still a fair number of cases where it's unclear that the message is a warning. Using prettyWarn prefixes with a yellow "Warning:". Handling logError would also be good, for similar reasons (and better layout!). Taking a look at the uses of logInfo would be good too, though probably a lot of those can be left unchanged.

May also be worth considering switching over to https://www.stackage.org/package/prettyprinter-ansi-terminal / https://www.stackage.org/package/prettyprinter . Though, from a cursory glance at the code, it isn't quite as efficient in its use of control codes (mine avoids doing a state reset for every color change). Thinking that might be a better choice for simplicity / more reliable if output gets interspersed.

I suppose it'd be better to switch to the prettyprinter package before adding a lot more use of prettyprinting, though the API should be quite similar since they are both in the wadler-leijen family.

@kadoban
Copy link
Collaborator

kadoban commented Aug 14, 2017

Cool, thanks for the info. I'll get looking in to things and start a WIP PR or a branch or something tonight. I'll probably try a place or two and then look at which prettyprinter to use once I have a basic idea of what I'm doing and how the interface seems like it's working and etc.

@kadoban
Copy link
Collaborator

kadoban commented Aug 24, 2017

What would be your preference for small, obvious changes to switch to pretty* functions? Should I do small PRs, one big PR, or just push to master?

Anything interesting I'll make a PR for, and the terminal detection of course, but I suspect it'll just be tedious doing PRs for every little change (or one big, never-ending PR could also be kind of bad for merge-conflict reasons). But I'm find doing whatever really.

@mgsloan
Copy link
Contributor Author

mgsloan commented Aug 24, 2017

I think my preference is medium sized PRs, but it's pretty much up to you, one big PR is alright as well. One way to do it might be to do a big PR, but push to it as you go.

Thanks for working on this, good stuff!

@kadoban
Copy link
Collaborator

kadoban commented Aug 24, 2017

I think my preference is medium sized PRs, but it's pretty much up to you, one big PR is alright as well. One way to do it might be to do a big PR, but push to it as you go.

Sure, that works. Maybe I'll do something like module-by-module for a while and see how that goes. I have a decent start at s/S/Package.hs I think, going to throw it in a WIP PR.

Thanks for working on this, good stuff!

Anytime, thanks for reviewing and advice!

@kadoban
Copy link
Collaborator

kadoban commented Aug 25, 2017

Won't have time until monday for more stack almost certainly. On monday, I'm planning on anything that comes up for #3384, and then terminal detection if nobody gets to it first. Pretty excited for terminal detection, that should be fun and satisfying. :) Edit: out of date scheduling stuff.

@mihaimaruseac
Copy link
Contributor

Now that #4205 is done, we can close this too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants