Jump to conversation
Unresolved conversations (0)
Nice work!

Nice work!

All of your conversations have been resolved.

Resolved conversations (5)
@jrohel jrohel Mar 18, 2025
I'm thinking of names. Actually the function does not return whether the tty is colored. It returns the user's preference of whether or not they want colors based on what they set with `set_colorized`. So on a colored `tty` it may return `false` and vice versa. But I can't think of a better name... `set_use_colors`, `get_use_colors` ... I don't know
Outdated
include/libdnf5-cli/tty.hpp
alimirjamali jrohel
Ali Mirjamali and Jaroslav Rohel
@jrohel jrohel Mar 18, 2025
The `colorized` variable is a private variable. It is not in the header file and is only accessed within that file. Please make it `static`.
Outdated
libdnf5-cli/tty.cpp
alimirjamali
Ali Mirjamali
@jrohel jrohel Mar 18, 2025
Valid values are: - `always`, `on`, `yes`, `1`, `true` - `never`, `off`, `no`, `0`, `false` - `auto`, `tty`, `if-tty` So we are fully compatible with DNF4. It also accepts all the values listed. But the truth is that the DNF4 man page only lists `always`, `never`, and `auto`. I am considering whether we want to ignore the other possible values in the documentation again. It may be practical to know about the possibility to write `--color=1` instead of `--color=always` and `--color=0` instead of `--color=never`.
doc/dnf5.8.rst
jrohel alimirjamali
Jaroslav Rohel and Ali Mirjamali
@jrohel jrohel Mar 18, 2025
Reading the value is done too early. The configuration file has not yet been loaded and processed by the plugins. Move this code block further. I suggest right after the `base.setup();` line.
Outdated
dnf5/main.cpp
alimirjamali
Ali Mirjamali
@jrohel jrohel Mar 18, 2025
Please remove this line. The constant value argument is only used for options with `get_has_value() == false`.
Outdated
dnf5/main.cpp
alimirjamali
Ali Mirjamali