-
Notifications
You must be signed in to change notification settings - Fork 182
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 terminal and color escape sequences #580
Conversation
7481807
to
4128e77
Compare
Looking good. |
I am personally not a fan using derived types. The ideal for this would be to use enumerations (@FortranFan would probably approve:), in fact that's what we ended up using in C++ that you linked above. We also have to initialize the terminal (especially on Windows). I don't know if there is a way to simplify this somehow. If not, then the present approach is fine. |
One question: this module talks about "colors", but these are specifically terminal colors, and in fact only "ansi escape sequences", but it seems all terminals now converges to support those, including Windows. Should this be perhaps named "terminal colors"? |
And since we require to setup a terminal (on Windows) even to just print colors, we might as well use this machinery for other terminal things, such as the goal of the I think this is within scope of stdlib (although I always struggle if we should create a separate package or add it to stdlib). |
I am not a user of terminal colors (yet), however upon reviewing the module I was wondering if abbreviating the components as type(fg_color24) :: my_color
my_color = fg_color24(r=34, g=12, b=10) ! vs
my_color = fg_color24(red=34, green=12, blue=10) |
Thanks for the feedback. Building something like I usually start building this kind of projects separately and than notice that simple things like |
I don't think the fact Fortran doesn't support proper enums should be a show stopper. Here is a small quote from the Lua community:
Derived types offer a nice and type-safe way to organize code, even if they might lead to reduced performance in some cases. For ANSI Escape sequences specifically, the other two options I see available are 1) using a character string, like the M_attr module does, or 2) use an array of 3 integers. (In principle you could encode all 24 bits in a single integer, but I guess it makes code less clear.) Would it be feasible to overload the |
I actually thought about overloading |
Here's the example @certik posted in #229: std::string text = "Some text with "
+ color(fg::red) + color(bg::green) + "red on green"
+ color(bg::reset) + color(fg::reset) + " and some "
+ color(style::bold) + "bold text" + color(style::reset) + ".";
std::cout << text << std::endl; The same can be achieved in Fortran. I guess using type(fg_color) :: green, red
print *, green // "Sentence in green" ! valid
print *, "Red sentence" // red ! invalid Edit: on second thought, overloading |
I agree. All I am advocating for is to consider and search for the simplest solution that involves the least amount of "layers" (such as derived types or even more OO approach). Btw, I am not convinced the C++ I approach I took is the best either. I guess the only approach I can think of is to have functions like That's why I CCed @FortranFan, because this use case is far from unique, I think this is actually quite common. |
Since Fortran now supports (some more than others) everything from procedural to functional to OOP programming there are a lot of options and all of us have different approaches (some of mine and others discussed in M_esc and to a lesser extent M_attr) but if going with OOP and user defined types I might want a string that can have attributes (bg color, fg color, blink, underline, ...) that I can set that I would otherwise use like the STRING type in stdlib but that on output I could optionally print with all the attributes applied; albeit if putting out a lot of short strings with similar attributes it would take some work to not produce redundant output when all the strings have a common attribute. I think some example programs solving some common use cases would help. Looks good given the approach taken, but unless you want to expand this to allow building panels and./or ASCII graphics it seems a little overkill; but since it works and something is needed and it has the attribute of being expandable to include positioning and clearing I am good with it. Having been using an ncurses interface and more recently M_attr I am having a hard time getting through the "not what I am used to" roadblock while trying it, but everything I have tried so far has worked. I personally need something I can easily read and write from an external file and turn off so I need more of an abstraction of the attributes I can easily embed in text files; but I think a lot of people just want an easy way to color some text and this works for that so I am good with this. |
@certik wrote Nov. 28, 2021 11:52 AM EDT:
Thanks @certik. I agree the use case here is hardly unique: scientific and technical computing is replete with the need to work with named constants with particular scopes in type-safe manner even in compute-intensive sections of codebases. The need here to work with color codes for terminal escape sequences is but one instance that some might consider is outside of hard-code number crunching but it should not trivialized. Nonetheless proper ENUMs are a bridge way too far for Fortran, the solution in the next revision Fortran 202X is far from optimal. Thus moving ahead with this Fortran stdlib item in sync with all your consensus is as good as it can get. |
@awvwgk is this ready for review? |
I think we have agreed that terminal escape sequences are generally in-scope for stdlib. The question now is how make best use of them in an actual application and I don't really have a definite answer, this touches points like:
I haven't touched point 3 here in this PR, but I think this is one of the most important points to cover. However, I don't think there is a best answer. The most practical solution I was able to come up with is here, using a derived type to hold all escape sequences and initialize them with empty string if color support is disabled, while deferring the decision of color usage to the user of the library. But I doubt this is the best strategy, since it requires explicit initialization. |
Point 3 is not a blocking issue for this PR to go IMO, it produces all the code escapes needed to color a text. We can just let the user of the module deal with it and provide helpers later on (E.g: a portable
|
@awvwgk Thank you for the answers. I can't answer your questions. However, I would suggest to finish this PR as is, and ask users some feedbacks regarding these 3 questions. With fpm, people could test it quite easily IMO. |
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.
Thank you for this addition. I reviewed the files and played with it locally. It works as expected and I think it can be merged as is, and we can add any higher-level functionality in later PRs.
I agree with @certik that we should look for the simplest and lowest-level approach, though I think the derived types here are appropriate. Alternatively, the color derived types could also be expressed as integer :: fg_color24(3)
arrays, but to me the current implementation is more intuitive.
What's not so intuitive to me is that we have two separate types fg_color24
and bg_color24
(and corresponding "enum" types) that are otherwise exactly the same. As I understand it, this is designed like this so that a generic to_string
can figure out whether to emit a background or foreground styling. As a user, I would prefer for colors to be just colors (e.g. type color24
), but use different functions (to_string_fg
, to_string_bg
, etc.) to apply different kind of stylings. In other words, in my mind foregroundness and backgroundness are not properties of a color, but a property of a styling operation. I hope that makes sense. But I also understand this is a matter of style, so I'm not opposed to the current approach.
One question that came up for me is if the color components of the derived types should be integer(int8)
(as that would make the types truly 24-bit :)). Memory footprint would be four times smaller (are there any use cases where the memory use would be important, i.e. large arrays of color values? I can't think of it. In general graphics sure, but in terminal? I don't know. Maybe somebody will start writing rich-color roguelikes in Fortran). The default constructor would be uglier (e.g. fg_color24(0_int8, 127_int8, 255_int8)
), but that could be easily fixed by providing a custom type constructor function. Another advantage could be type casting: Currently if you pass values out-of-range (e.g. < 0 or > 255) the style simply won't work but the program would otherwise be correct. But if you pass out-of-range values to int8
, a compiler could emit helpful warnings if enabled.
@awvwgk Do you think there is sufficient interest and feedback to wrap up and merge this PR? If yes, I'd be happy to help resolve the merge conflicts. |
Didn't have time to look into this PR for a while now. Working with color support for a bit I was somewhat unhappy with the API defined here for my projects, as I usually need a more high-level interface to turn off the color printout. The overall structure defined here might be just right to build such a low level interface. I'm fine with finishing this PR and merging it as is, still there is some way to go. |
Co-authored-by: Ian Giestas Pauli <[email protected]>
Just rebased and renamed the derived type to |
|
Co-authored-by: Ivan Pribec <[email protected]>
doc/specs/stdlib_terminal_color.md
Outdated
#### Argument | ||
|
||
``lval``: Style, foreground or background code of ``ansi_code`` type or a character string, | ||
this argument is ``intent(in)``. | ||
``rval``: Style, foreground or background code of ``ansi_code`` type or a character string, | ||
this argument is ``intent(in)``. | ||
|
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.
The way this is currently written one might wrongly interpret that codes can be "added" using the the // operator. Only after I checked the interface I realized this was not the case.
I've got no good suggestions how to amend this.
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.
A few comments:
- I'm not 100% convinced by the naming of the module and constants. The derived type name
ansi_code
is good, but I wonder ifstdlib_ansi
orstdlib_ansi_codes
would be better for the module name? For the colors, I feel like the "color" word is just extra characters. There is the actual color name in the variable name anyway, and it could be just stated in the documentation that parameters prepended withfg_
andbg_
are colors. A second consideration I had was to prependansi_
to all named parameters for extra clarity, but I'm not fully decided if I like it or not. - I find the right-overwriting rule of the addition operator a bit nonintuitive the way it reads in the documentation. I suppose the correct usage pattern is to add up to one code from each group (fg, bg, and style). In that case the over-writing direction doesn't actually matter, unless you are "recycling" code variables. The graphics language Asymptote offers something similar for combining different colored pens:
Pens may be added together with the nonassociative binary operator +. This will add the colors of the two pens. All other non-default attributes of the rightmost pen will override those of the leftmost pen. Thus, one can obtain a yellow dashed pen by saying dashed+red+green or red+green+dashed or red+dashed+green.
- It would be interesting to look at some micro-optimizations for the ANSI code to string translation step in the future. Granted, colors are typically not used in performance critical sections. I'm just being pedantic.
- We can offer a functional API in addition to the operator one. This would reset the sequence by default. There could also be a second function with a logical flag for resetting.
pure function colorize(code, str)
type(ansi_code), intent(in) :: code
character(len=*), intent(in) :: str
character(len=:), allocatable :: colorize
colorize = code // str // style_reset
end function
- Should string type be supported to? Can also be deferred to a new PR.
NO_COLOR
could be easily supported in the future by changing the behavior ofto_string_ansi_code
.- Perhaps I missed the discussion, but what was the reason for making the structure constructor private? Is it to force users to initialize it explicitly after the declaration section? This ties back to comment 2).
Overall, I think it's a great how this new "low-level" interface turned out. I say low-level even if derived types and operator overloading is involved. After-all this is the "standard" library, and compiler vendors can choose to implement this in a more primitive fashion if they really want to. In that case ansi_code
should probably be a sequence type to prevent access of the internals through inheritance.
Co-authored-by: Ivan Pribec <[email protected]>
Co-authored-by: Ian Giestas Pauli <[email protected]>
Co-authored-by: Ivan Pribec <[email protected]>
Naming is always hard,
This module is developed with support for easily disabling colors in mind, if you do not initialize your escape sequence with a color you get the
What should happen if the user initializes the escape sequence with
Can be supported as well, should only require two new procedures.
Isn't |
@awvwgk If you are satisfied with this PR, I suggest that you merge it, such that users can test it, e.g., through |
I'm already using the module in a project and will soon also adopt it in TOML Fortran, the design works nicely so far for my usage. Now that we have the tree-shaking in fpm I could adopt it TOML Fortran via stdlib, but I'm still hesitant to limit myself in the compiler choice (I have to support GCC 5 and NAG 7 which are both not supported with stdlib but currenly work with TOML Fortran). |
Thanks Sebastian for the contribution and all reviewers. I'll merge now. |
true color (24-bit) typesQuestions:
Related