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 target to status output #343

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mrtnpaolo
Copy link

Howdy, this patch adds the target to the status output.

This is especially useful when one has multiple ghcid instances running (e.g. working at the same time on an executable and libraries it depends on.)

I put in a couple extra headings for the errors and warnings to give the target a reasonable place to appear in and I matched GHCi's color scheme for those.

One thing I'm not sure about is what the window title format should be: maybe it's good enough as it is since it shows the project? I also do not have a windows machine to try and get a feel.

Would appreciate feedback!
Thanks.

Copy link
Owner

@ndmitchell ndmitchell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding the targets to the all good message and the status bar seems reasonable, I'd be happy with that using a few cleanups. Less convinced about warnings/errors, since that is more precious real estate.

I wonder if the targets are blank if the basename of the current directory might be useful to include instead?

allGoodMessage :: String
allGoodMessage = setSGRCode [SetColor Foreground Dull Green] ++ "All good" ++ setSGRCode []
allGoodMessage :: [String] -> String
allGoodMessage target = setSGRCode [SetColor Foreground Dull Green] ++ "All good " ++ setSGRCode [] ++ "in " ++ concat target
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If target is blank, as it often will be, this will look really odd - i suspect you need to hide the "in " in that case.

: concatMap printEval evals
prettyOutput _ _ xs evals = concatMap loadMessage xs ++ concatMap printEval evals
prettyOutput Options{..} _ _ xs evals =
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern is that if you have only a handful of lines (e.g. 5) then in the case where you have errors/warnings, you've added 1 header line and decreased the amount of error/warning to display - potentially by a meaningful amount.

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.

2 participants