Commits on Source (26)
-
traces back to 2001 cfbe8690
34fe1c70 -
6c9127b7
-
where negative variants of deprecated boolean options are used, the warning would previously specify the positive variant instead, which is silly and potentially confusing to a user.
6b7eee26 -
clarifies that this is an error message; helps users find errors by searching output for "error" in some situations maybe?; better consistency; kinda expected; and combined with later work to add colour highlighting, improves UX by helping to highlight problems in terminal output.
3cf3bb4c -
unnecessary and ugly for the vlc media player use case, hopefully also unnecessary for other libvlc use cases. note that for now at least, this error output is followed immediately with another line that includes "vlc" in it.
0575b068 -
77282fb8
-
for options that need an accompanying "value", users can either supply it within the same command-line argument as the option itself, like `--foo=bar`, or within the next argument, like `--foo bar`. (for short options this would be `-fbar` and `-f bar` respectively). it can be argued that it is possibly confusing to users to thus speak of a missing "argument" when such an option "value" is missing. let's speak of a missing option "value" instead.
f104f95e -
getopt can return ':' and '?' chars to indicate special conditions. there is nothing to prevent such characters being assigned to options, and thus there is a risk of misinterpretation, for instance misinterpreting a sign of a missing option value error condition as a match for such a short option. this change ensures that in the hypothetical event someone is stupid enough to try to assign one of these chars to an option, we prevent situations of misinterpretation of error conditions, at the acceptable expense of that short option not being allowed to work at all.
251f67ec -
null = obvious ':' and '?' have special getopt meaning
5ba65ea2 -
58244c94
-
...to prepare for text translation and colour labels. (i am not certain that tacking the option onto the end, as we were, will be suitable for all translations). i've avoided using allocation to construct the option name portion of the output, since i expect that would not be liked, and i'm not convinced about it myself. it can always be reconsidered later however.
4417e1fb -
0cacb2e6
-
the terminal control code stuff is relevant to (and wanted by me for) cmdline.c
863a19ed -
- replaced use of `GRAY` and `WHITE` for resetting with `RESET` and `RESET_BOLD` respectively. - addressed hidden use of bold by creating separate colour and colour+bold defines. - added a `TS_` prefix to help clarify that the defines, where they are used, are terminal sequences.
077aa681 -
6900989f
-
e4fa5c37
-
improves UX by helping to highlight problems in terminal output. for now, as with the rest of the codebase, we rely upon only a simple `isatty()` check for automatic colouring decisions, which does not work for Windows. implementing `isatty()` for Windows to could perhaps be tackled later. --- note that user control over use of colour in terminal output via the use of `--color`/`--no-color` options is deliberately ignored with respect to the use of colour within error and warning output in response to any problems encountered whilst processing the supplied command-line arguments themselves (unknown option error; missing option value error; obsolete option warning), having decided that this was the best choice. this was questioned in review of this work, so i have expanded/documented my thoughts on it here below. 1. default behaviour depends upon whether or not a detection routine determines that output is connected to a tty. if so then we choose to use colour in the hope that most users will appreciate the highlighting of problems. if not, i.e. output is thus being redirected into a log file or another application, we choose to not use colour because embeddeding terminal sequences into the output in such situations risks causing confusion due to the likely possibility of them not being consumed as intended. the only situations where users may want or need to change this default behaviour, are when either: a. they happen to dislike the use of colour. b. they want to force use of colour on Windows, due to current lack of a tty detection mechanism. the latter of course becomes redundant as soon as we implement an `isatty()` for Windows. it is reasonable in my opinion to fail to honour such a user preference for the limited circumstances of encountering a problem with the very data-set in which the user communicates that preference. never-the-less, let's briefly explore the possible solutions that could in theory be implemented to try to honour such user preference for such output. 2. solution 1: "pre-scanning" the concept was raised of "pre-scanning" the set of arguments to try to learn the user's indicated preference ahead of properly processing them, such that we can ensure that it is obeyed if/when outputting such an error/warning for a problem encountered in undertaking such processing. i.e. essentially walking through the argument list looking out for arguments matching a few fixed strings like "--color", and thus taking note of the preference before we begin the proper argument processing work. it should be understood that the rules of proper argument processing dictate that you cannot with certainty know the nature of an argument until you have processed fully all of those preceeding it. options that take data values can be used with the value given within the same argument or supplied within the next one, and so you never know if an arbitrarily given argument must be consumed as a data value rather than treated as a possible option until you've processed all proceeding arguments; a special "early terminator" (`--`) argument indicates that all subsequent arguments must be treated as generic arguments (that must not be interpretted as options); and an unknown option argument brings processing to a halt since it is not possible to correctly interpret the nature of anything subsequently given. despite this, practically speaking we could get away with pre-scanning - there are no "short" option forms of these toggle options to complicate things; and making simple assumptions that arguments matching "--color" or "--no-color" (or "--nocolor") are actual uses of those options is pretty reasonable along with assuming that a "--" argument is an actual early terminator, since such assumptions are almost always likely to be correct except in certain very unusual circumstances of broken CLI use or from users deliberately looking for flaws. however, i do not feel that it is worth adding a chunk of code just to essentially ensure that a hypothetical and presumably rare CLI user who dislikes colour will not see it should they happen to make a mistake in their CLI usage. (it might be noted that the vlc binaries do similarly flawed scans to steal one or two options, but that's not readily avoidable). 3. solution 2: "updating as we go": as an alternative concept, we could, upon successfully processing each option, check if the option is `--color` or `--no-color` and if so, update the `color` boolean accordingly, such that if we run into a problem and thus need to output an optionally coloured error or warning label, we might thus obey the user's preference when doing so. however, we have equal chance that we do the opposite since there could be a later argument that we did not yet get to that flips the preference the other way (e.g. `vlc --no-color --foo --color`). in a worst case scenario the user might end up with a mix of coloured and non-coloured output that's messy and possibly confusing. again, i do not feel that it is worth implementing code to do this, to introduce an inefficiency into the option processing for such little benefit, especially with the risk of this creating a mess of inconsistency in some cases. we (i) should endeavour to introduce `isatty()` support for Windows such that Windows CLI users get the benefits of colour highlighting, but i feel that it is perfectly reasonable to take a stance of not bothering to try to enforce user no-color preference upon argument processing errors/warnings.
e77ae919 -
for subsequent use with suggestion matching.
8fb6f8d0 -
to be used with suggestion matching. this implementation is based upon the implementation from the `strsim` Rust crate, authored by Danny Guo, available at [1]; more specifically the (as yet un-merged) optimised copy authored by myself, available at [2]. the code is available under the MIT license. one implementation difference is that we use floats rather than doubles since we only intend to use this for suggestion matching in unknown option error messages and we don't need the extra precision. [1]: https://github.com/dguo/strsim-rs [2]: https://github.com/dguo/strsim-rs/pull/31
86f9e53b -
i.e. if a user tries a long option which does not exist, if it is a close enough match to one or more available options, the error message can include the best match as a helpful suggestion.
4ad4a388 -
old: "Try `vlc --help' for more information." new: "For more information try `--help`" in experiments, the new text proved much cleaner. it becomes yet more clean, having dropped "vlc" from `vlc --help`, when we then drop the quotes around the remaining `--help` in the commit after next. note, we now no longer have "vlc" in any of the error output. this is not a problem for the vlc app itself obviously; i hope it is not a problem for any other libvlc use cases.
8fdcd829 -
inspired by Rust's cargo output, IMO this adds a nice little enhancement to UX along with the colour error/warn labels.
3df5e077 -
experimenting with this i decided that it is cleaner to not use them, for valid option names at least. they are still used for unknown option quoting for instance, and where we are quoting multiple CLI arguments like `--foo <VAL>` or `vlc --foo`.
6319b067 -
creating the option label with `asprintf()` up front allows us to tidy things up quite a bit, removing most notably the duplication of strings that differ only between char and string format items.
4e691ef9 -
i don't believe we have any short options assigned to float options currently, but we should have this case here prepared to handle it.
5802f57c -
to be clear about what we do for them.
dbf81e9a
Showing
- src/Makefile.am 5 additions, 0 deletionssrc/Makefile.am
- src/config/ansi_term.h 122 additions, 0 deletionssrc/config/ansi_term.h
- src/config/cmdline.c 68 additions, 18 deletionssrc/config/cmdline.c
- src/config/getopt.c 0 additions, 1 deletionsrc/config/getopt.c
- src/config/help.c 20 additions, 26 deletionssrc/config/help.c
- src/config/jaro_winkler.c 167 additions, 0 deletionssrc/config/jaro_winkler.c
- src/config/vlc_getopt.h 7 additions, 0 deletionssrc/config/vlc_getopt.h
- src/config/vlc_jaro_winkler.h 43 additions, 0 deletionssrc/config/vlc_jaro_winkler.h
- src/modules/entry.c 3 additions, 1 deletionsrc/modules/entry.c
- src/test/jaro_winkler.c 101 additions, 0 deletionssrc/test/jaro_winkler.c
src/config/ansi_term.h
0 → 100644
src/config/jaro_winkler.c
0 → 100644
src/config/vlc_jaro_winkler.h
0 → 100644
src/test/jaro_winkler.c
0 → 100644