2020wk13 more more(1) changes #1003
Conversation
|
An additional commit |
|
I noticed an issue in |
text-utils/more.c
Outdated
| for (opt = 0; opt < as_argc; opt++) { | ||
| int move = 0; | ||
|
|
||
| if (as_argv[opt][0] == '-' && isdigit(as_argv[opt][1])) { |
There was a problem hiding this comment.
Maybe use isdigit_string() instead of isdigit()
There was a problem hiding this comment.
Fixed to use isdigit_string() in ffb1f57.
| if (move) { | ||
| as_argc--; | ||
| memmove(as_argv + opt, as_argv + opt + 1, sizeof(char *) * (as_argc - opt)); | ||
| opt--; |
There was a problem hiding this comment.
maybe it would be better to add to include/env.h inline function remove_arg(char **arg) and use it in sanitize_env() as well as in more(1).
text-utils/more.c
Outdated
| ssize_t i, ilen; | ||
| struct number_command cmd = { .key = more_kc_unknown_command }; | ||
|
|
||
| if ((ilen = read(STDERR_FILENO, &input, sizeof(input))) <= 0) |
There was a problem hiding this comment.
why stderr? why not stdin?
There was a problem hiding this comment.
This will be dealt in upcoming follow up pull request that will add up / down key commands.
text-utils/more.c
Outdated
| return cmd; | ||
| } else if (!memcmp(input, "\x1b\x5b\x36\x7e\x00", 5)) { /* page down */ | ||
| cmd.key = more_kc_skip_forward; | ||
| return cmd; |
There was a problem hiding this comment.
Not sure, but would it be better to use getch() from ncurses and KEY_NPAGE and KEY_PPAGE (etc)? It seems strange to hardcode it to the code.
There was a problem hiding this comment.
Another thing I will address in upcoming follow up. That said I will look into using getch().
There was a problem hiding this comment.
Oh, but to get getch() working more(1) would require initscr() and that will lead to terminal window. That is not how I imagine more(1). The differentiating factor of more vs less is if terminal window is used or not.
|
What about to split it to two PR, one for real clean ups and another PR for new features like the libmagic and page-up/down? :-) |
A function to remove an command-line option argument, or environment variable. Requested-by: Karel Zak <kzak@redhat.com> Reference: util-linux#1003 (comment) Signed-off-by: Sami Kerola <kerolasa@iki.fi>
|
Both libmagic and page up/down changes are removed from this pull request. They will be part of follow up sometime later. |
And be a little more complete all the allocations that can be released are, but there is a small catch. As mentioned in ncurses FAQ some leaks are intentional, and that's the way they are. Reference: https://invisible-island.net/ncurses/ncurses.faq.html#config_leaks Reference: http://man7.org/linux/man-pages/man3/_nc_free_tinfo.3x.html Signed-off-by: Sami Kerola <kerolasa@iki.fi>
Only the terminal of the child process, that is the command being executed, needs the reset. Signed-off-by: Sami Kerola <kerolasa@iki.fi>
As a minor robustness improvement use ftello() to find out file position rather than add and substract from old value. Signed-off-by: Sami Kerola <kerolasa@iki.fi>
The sys/ttydefaults.h can be used for CTRL() and rubout that is CERASE. For alarm it's best to keep things simple and call printf() alarm modifier. QUIT was not in use, so it is just deleted. Signed-off-by: Sami Kerola <kerolasa@iki.fi>
These functions did similar things, so there is no need for both of them. Signed-off-by: Sami Kerola <kerolasa@iki.fi>
If streams need to be flushed do not try to be clever, just flush all of them. That lead to finding unnecessary print out in run_shell() that is removed in this commit. Signed-off-by: Sami Kerola <kerolasa@iki.fi>
The wait() is now a little more robust by being more tolerate rogue SIGCHLDs and unblocked signals. The repeated fork() and sleep() is removed, if first try does not success give up without delay to provide user timely feedback. Signed-off-by: Sami Kerola <kerolasa@iki.fi>
When suspending only the more process. Sending signal to process group makes signal destination unnecessarily vague. After the suspend is over SIGCONT is expected, and it needs to ensure output terminal settings are what more needs. Signed-off-by: Sami Kerola <kerolasa@iki.fi>
Linux terminal handles underlining just fine without more executable needing to do anything extra. Signed-off-by: Sami Kerola <kerolasa@iki.fi>
This commit also includes fix to how initial skip lines and search are instructed in the code. Earlier version was pretty close impossible to make work with getopt_long() and had minor flaw - if both initial skip lines and search were defined at the same time the skipping did not happen. That is now corrected. Signed-off-by: Sami Kerola <kerolasa@iki.fi>
Moving backwards has worked fine until reaching start of file. At that point more printout had one line too much in output causing more seemingly never to be able to reach the start. That is now fixed, with clean ups to skip_backwards() making it less confusing. Signed-off-by: Sami Kerola <kerolasa@iki.fi>
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
A function to remove an command-line option argument, or environment variable. Requested-by: Karel Zak <kzak@redhat.com> Reference: util-linux#1003 (comment) Signed-off-by: Sami Kerola <kerolasa@iki.fi>
|
Thanks! |
Aim was to introduce page and arrow up/down keys to more(1), but that also required merging colon_command() and more_key_command() functions. The more_key_commands enum is pointless from computers point of view. The command identification performed in read_command() inline with more_key_command() execution -- but that would be hard for humans, and source code ought to serve both parties. Reference: util-linux#1003 Signed-off-by: Sami Kerola <kerolasa@iki.fi>
Aim was to introduce page and arrow up/down keys to more(1), but that also required merging colon_command() and more_key_command() functions. The more_key_commands enum is pointless from computers point of view. The command identification performed in read_command() inline with more_key_command() execution -- but that would be hard for humans, and source code ought to serve both parties. Reference: util-linux#1003 Signed-off-by: Sami Kerola <kerolasa@iki.fi>
Aim was to introduce page and arrow up/down keys to more(1), but that also required merging colon_command() and more_key_command() functions. The more_key_commands enum is pointless from computers point of view. The command identification performed in read_command() inline with more_key_command() execution -- but that would be hard for humans, and source code ought to serve both parties. Reference: #1003 Signed-off-by: Sami Kerola <kerolasa@iki.fi>
Continuation to series of more(1) changes. Notable changes in this change set are use of libmagic to identify non-textual files, removal of underlining handling that is not needed on a modern linux terminals that will handle underlining just fine, and addition of long options.