On 18 May 2014 15:05, Sami Kerola <kerolasa@xxxxxx> wrote:
> Sami Kerola (17):
> setterm: clean up includes
> setterm: use getopt_long_only() for option parsing
> setterm: use string utils to numeric parsing
> setterm: move show_tabs() and screendump() functions
> setterm: remove usage comment segment
> setterm: add option control structure
> setterm: add init_terminal() to make main() shorter
> setterm: clean up screendump()
> setterm: remove devfs and /dev/vcsa0 support
> setterm: make -msglevel 0 to work as is did earlier
> setterm: correct usage() bright color argument
> setterm: improve perform_sequence() coding style
> setterm: tell user when options does not effect
> setterm: improve error messages
> setterm: various visual terminal effects are not console specific
> setterm: mark some options to be exclusive with each other
> setterm: add set_blanking() action
Hi Karel & thank you for feedback, here are my collected improvements.
0002 setterm-use-getopt_long_only-for-option-parsing
> + errx(EXIT_FAILURE, "duplicate use of an option");
> NLS.. _("duplicate use of an option")
Fixed.
0003 setterm-use-string-utils-to-numeric-parsing
> Why not strtos32_or_err() from lib/strutils.c ?
Because I did not think. Fixed.
0006 setterm-add-option-control-structure
> + memset(&ctl, 0, sizeof (ctl));
>
> The memset() call is unnecessary. It's enough to set any struct member
> to zeroize whole struct, etc:
>
> struct setterm_control ctl = { .opt_te_terminal_name = NULL };
I did not know, and then I went to find out more. Universal zero
initializer seem to be an option many recommend, so I'll switch to this
format.
struct setterm_control ctl = { 0 };
This is what the standard tells, just in case someone finds util-linux
mail archive and needs a reference.
http://c0x.coding-guidelines.com/6.7.8.html
0009 setterm-remove-devfs-and-dev-vcsa0-support
> Sooo.. do we really need read_error()? ;-)
Yup, not needed.
0011 setterm-correct-usage-bright-color-argument
> too many duplicate information, it would be better to use
>
> _(" -foreground <default>|<color>\n")
> ...
> _(" -hbcolor <bright> <color>\n")
>
> and describe below in usage():
>
> _(" <color> means black, blue, ...")
Fixed. The fix has tiny usage() error. It gives impression grey might
work as --foreground color, but it does send an error. Oh well, users
will need to read friendly manual if they are confused.
0013 setterm-tell-user-when-options-does-not-effect
> Please use complete long options in error messages ("--repeat" rather
> than "-repeat").
Fixed. And if the the errors are expected too have double dash long
options so should the usage(). That is done in commit below.
https://github.com/kerolasa/lelux-utiliteetit/commit/fb27f91cac9702ad1858d782dd840d5868547423
Here are the updated pull details. I assume the release v2.25 is done
before merge, and if so I will keep on rebasing the changes until the
merge.
git://github.com/kerolasa/lelux-utiliteetit.git 2014wk19
--
Sami Kerola
http://www.iki.fi/kerolasa/
--
To unsubscribe from this list: send the line "unsubscribe util-linux" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
