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

Fix console formatting with colors #176

Merged
merged 1 commit into from
Jan 10, 2022

Conversation

AltGr
Copy link
Contributor

@AltGr AltGr commented Jan 10, 2022

closes #174

@AltGr
Copy link
Contributor Author

AltGr commented Jan 10, 2022

This should do it, at least for the main printers. A few remarks:

  • the trick is to use either pp_print_as to specify the length in screen columns, when adding the color escapes; alternatively you can use @<n> if you statically know the length to print (e.g. let pp_red_escape fmt = Format.fprintf fmt "@<0>%s" "\027[31m".
  • Many code paths go to strings before printing (Format.asprintf) so the trick won't work there; I assumed that this cases correspond to cases where you don't need formatting anyway.
  • I noticed many cases of Format.fprintf fmt "%a" f x, which is more simply written f fmt x.
  • If you are interested it's possible to define specific tags in Format. Probably some libs do this already... handling nesting properly is tricky.

Copy link
Contributor

@denismerigoux denismerigoux left a comment

Choose a reason for hiding this comment

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

It works, it's so satisfying! Plus the fix was relatively minor, congrats @AltGr!

@@ -131,6 +131,11 @@ let time : float ref = ref (Unix.gettimeofday ())
let print_with_style (styles : ANSITerminal.style list) (str : ('a, unit, string) format) =
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you deprecate and remove this one?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well it's still used in many places where printing doesn't go through Format, I think it can stay

@denismerigoux
Copy link
Contributor

Many code paths go to strings before printing (Format.asprintf) so the trick won't work there; I assumed that this cases correspond to cases where you don't need formatting anyway.

I'm afraid that a lot of these paths that use a string before printing are because of me unable to use Format correctly...

@denismerigoux denismerigoux merged commit fcde859 into CatalaLang:master Jan 10, 2022
@AltGr
Copy link
Contributor Author

AltGr commented Jan 10, 2022

Well Format is pretty good for formatting code, but has some inflexibilities that make it not a silver bullet for printing all kinds of messages, I am not among the ones that want everything to go through it everywhere (although as you noticed, mixing Format and non-Format printers for all datatypes soon becomes cumbersome). For example, printing a table with Format is basically impossible.

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.

Format and ANSITerminal interacting poorly
2 participants