[PATCH 0/2] more(1) use libmagic and make page up/down and arrow keys work

[PATCH 0/2] more(1) use libmagic and make page up/down and arrow keys work

[Sami Kerola]

Hello,

Here are two changes to more(1) that where requested in earlier github
pull[1] request to get review in maillist, so here we go.

[1] https://github.com/karelzak/util-linux/pull/1003

First one of the changes will link more(1) with libmagic so that various
binary formats can be detected with effect the pager will not mess up
termial.

Second change is perhaps a little bit more exciting.  It will add page-up,
page-down, arrow-up, and arrow-down movements to more(1).  This pager has
known now to scroll back and forth since long time ago, but most of the
people do not seem to know key bindings so lets help users by assigning the
most obvious buttons the way one might expect them to work. 

These changes can also be found from my github repo.

----------------------------------------------------------------
The following changes since commit 0d855a83095adc953cc9a0df8998a2f916242695:
  agetty: save the original speed on --keep-baud (2020-05-04 12:27:42 +0200)
are available in the Git repository at:
  git://github.com/kerolasa/util-linux.git more-features
for you to fetch changes up to 0cfa98c59a743a8c5dc0bec24bb46220a967ef85:
  more: make page and arrow up/down to update view (2020-05-06 19:57:56 +0100)
----------------------------------------------------------------

Sami Kerola (2):
  more: use libmagic to identify binary files
  more: make page and arrow up/down to update view

 configure.ac             |  12 ++
 text-utils/Makemodule.am |   2 +-
 text-utils/more.c        | 431 ++++++++++++++++++++++++---------------
 3 files changed, 284 insertions(+), 161 deletions(-)

-- 
2.26.2

[PATCH 1/2] more: use libmagic to identify binary files

[Sami Kerola]

As the old commend said: 'This code would best be shared with the file(1)
program or, perhaps, more should not try to be so smart'.  Now at configure
time one can choose whether more(1) is sharing code with file(1), or not.

Addresses: http://bugs.debian.org/139205
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 configure.ac             | 12 ++++++++++
 text-utils/Makemodule.am |  2 +-
 text-utils/more.c        | 52 +++++++++++++++++++++++++---------------
 3 files changed, 46 insertions(+), 20 deletions(-)

diff --git a/configure.ac b/configure.ac
index 1428dc893..8f50e8102 100644
--- a/configure.ac
+++ b/configure.ac
@@ -1501,6 +1501,18 @@ AS_IF([test "x$with_libz" = xno], [have_z=no], [
   AC_CHECK_LIB([z], [crc32], [have_z=yes], [have_z=no])
 ])
 
+AC_ARG_WITH([libmagic],
+  AS_HELP_STRING([--without-libmagic], [compile without libmagic]),
+  [], [with_libmagic=auto]
+)
+AS_IF([test "x$with_libmagic" = xno], [have_magic=no], [
+  AC_CHECK_LIB([magic], [magic_open], [
+    AC_DEFINE([HAVE_MAGIC], [1], [Define to 1 if you have the libmagic present.])
+    MAGIC_LIBS="-lmagic"
+  ])
+])
+AC_SUBST([MAGIC_LIBS])
+
 
 AC_ARG_ENABLE([cramfs],
   AS_HELP_STRING([--disable-cramfs], [do not build fsck.cramfs, mkfs.cramfs]),
diff --git a/text-utils/Makemodule.am b/text-utils/Makemodule.am
index 3f10934e9..6c4b52082 100644
--- a/text-utils/Makemodule.am
+++ b/text-utils/Makemodule.am
@@ -80,7 +80,7 @@ bin_PROGRAMS += more
 dist_man_MANS += text-utils/more.1
 more_SOURCES = text-utils/more.c
 more_CFLAGS = $(AM_CFLAGS) $(BSD_WARN_CFLAGS)
-more_LDADD = $(LDADD) libcommon.la
+more_LDADD = $(LDADD) $(MAGIC_LIBS) libcommon.la
 if HAVE_TINFO
 more_LDADD += $(TINFO_LIBS)
 more_LDADD += $(TINFO_CFLAGS)
diff --git a/text-utils/more.c b/text-utils/more.c
index 69a0b89f6..014b40a6c 100644
--- a/text-utils/more.c
+++ b/text-utils/more.c
@@ -74,13 +74,17 @@
 # include <term.h>
 #endif
 
-#include "env.h"
+#ifdef HAVE_MAGIC
+# include <magic.h>
+#endif
+
 #include "strutils.h"
 #include "nls.h"
 #include "xalloc.h"
 #include "widechar.h"
 #include "closestream.h"
 #include "rpmatch.h"
+#include "env.h"
 
 #ifdef TEST_PROGRAM
 # define NON_INTERACTIVE_MORE 1
@@ -157,6 +161,9 @@ struct more_control {
 	int last_key_arg;		/* previous key command argument */
 	int last_colon_command;		/* is a colon-prefixed key command */
 	char *shell_line;		/* line to execute in subshell */
+#ifdef HAVE_MAGIC
+	magic_t magic;			/* libmagic database entries */
+#endif
 	unsigned int
 		bad_stdout:1,		/* true if overwriting does not turn off standout */
 		catch_suspend:1,	/* we should catch the SIGTSTP signal */
@@ -350,19 +357,24 @@ static void print_separator(const int c, int n)
 	putchar('\n');
 }
 
-/* magic --
- *	check for file magic numbers.  This code would best be shared
- *	with the file(1) program or, perhaps, more should not try to be
- *	so smart. */
-static int check_magic(FILE *f, char *fs)
+/* check_magic -- check for file magic numbers. */
+static int check_magic(struct more_control *ctl, char *fs)
 {
+#ifdef HAVE_MAGIC
+	const char *mime_encoding = magic_descriptor(ctl->magic, fileno(ctl->current_file));
+
+	if (!mime_encoding || !(strcmp("binary", mime_encoding))) {
+		printf(_("\n******** %s: Not a text file ********\n\n"), fs);
+		return 1;
+	}
+#else
 	signed char twobytes[2];
 
 	/* don't try to look ahead if the input is unseekable */
-	if (fseek(f, 0L, SEEK_SET))
+	if (fseek(ctl->current_file, 0L, SEEK_SET))
 		return 0;
 
-	if (fread(twobytes, 2, 1, f) == 1) {
+	if (fread(twobytes, 2, 1, ctl->current_file) == 1) {
 		switch (twobytes[0] + (twobytes[1] << 8)) {
 		case 0407:	/* a.out obj */
 		case 0410:	/* a.out exec */
@@ -376,7 +388,8 @@ static int check_magic(FILE *f, char *fs)
 			return 1;
 		}
 	}
-	fseek(f, 0L, SEEK_SET);	/* rewind() not necessary */
+	fseek(ctl->current_file, 0L, SEEK_SET);	/* rewind() not necessary */
+#endif
 	return 0;
 }
 
@@ -394,23 +407,17 @@ static void checkf(struct more_control *ctl, char *fs)
 	    (fstat(fileno(ctl->current_file), &st) != 0)) {
 		if (ctl->clear_line_ends)
 			putp(ctl->erase_line);
-		warn(_("stat of %s failed"), fs);
-		ctl->current_file = NULL;
+		warn(_("cannot open %s"), fs);
 		return;
 	}
+#ifndef HAVE_MAGIC
 	if ((st.st_mode & S_IFMT) == S_IFDIR) {
 		printf(_("\n*** %s: directory ***\n\n"), fs);
 		ctl->current_file = NULL;
 		return;
 	}
-	ctl->current_line = 0;
-	ctl->file_position = 0;
-	if ((ctl->current_file = fopen(fs, "r")) == NULL) {
-		fflush(stdout);
-		warn(_("cannot open %s"), fs);
-		return;
-	}
-	if (check_magic(ctl->current_file, fs)) {
+#endif
+	if (check_magic(ctl, fs)) {
 		fclose(ctl->current_file);
 		ctl->current_file = NULL;
 		return;
@@ -704,6 +711,9 @@ static void reset_tty(struct more_control *ctl)
 /* Clean up terminal state and exit. Also come here if interrupt signal received */
 static void __attribute__((__noreturn__)) more_exit(struct more_control *ctl)
 {
+#ifdef HAVE_MAGIC
+	magic_close(ctl->magic);
+#endif
 	reset_tty(ctl);
 	if (ctl->clear_line_ends) {
 		putchar('\r');
@@ -1906,6 +1916,10 @@ int main(int argc, char **argv)
 
 	initterm(&ctl);
 
+#ifdef HAVE_MAGIC
+	ctl.magic = magic_open(MAGIC_MIME_ENCODING | MAGIC_SYMLINK);
+	magic_load(ctl.magic, NULL);
+#endif
 	prepare_line_buffer(&ctl);
 
 	ctl.d_scroll_len = ctl.lines_per_page / 2 - 1;
-- 
2.26.2

[PATCH 2/2] more: make page and arrow up/down to update view

[Sami Kerola]

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: https://github.com/karelzak/util-linux/pull/1003
Signed-off-by: Sami Kerola <kerolasa@iki.fi>
---
 text-utils/more.c | 379 +++++++++++++++++++++++++++++-----------------
 1 file changed, 238 insertions(+), 141 deletions(-)

diff --git a/text-utils/more.c b/text-utils/more.c
index 014b40a6c..b69fa5c5b 100644
--- a/text-utils/more.c
+++ b/text-utils/more.c
@@ -93,6 +93,11 @@
 #define BACKSPACE	"\b"
 #define CARAT		"^"
 
+#define ARROW_UP	"\x1b\x5b\x41"
+#define ARROW_DOWN	"\x1b\x5b\x42"
+#define PAGE_UP		"\x1b\x5b\x35\x7e"
+#define PAGE_DOWN	"\x1b\x5b\x36\x7e"
+
 #define MIN_LINE_SZ	256	/* minimal line_buf buffer size */
 #define ESC		'\033'
 #define SCROLL_LEN	11
@@ -121,6 +126,35 @@
 #define TERM_STANDARD_MODE        "smso"
 #define TERM_STD_MODE_GLITCH      "xmc"
 
+/* Used in read_command() */
+typedef enum {
+	more_kc_unknown_command,
+	more_kc_colon,
+	more_kc_repeat_previous,
+	more_kc_backwards,
+	more_kc_jump_lines_per_screen,
+	more_kc_set_lines_per_screen,
+	more_kc_set_scroll_len,
+	more_kc_quit,
+	more_kc_skip_forward,
+	more_kc_next_line,
+	more_kc_clear_screen,
+	more_kc_previous_search_match,
+	more_kc_display_line,
+	more_kc_display_file_and_line,
+	more_kc_repeat_search,
+	more_kc_search,
+	more_kc_run_shell,
+	more_kc_help,
+	more_kc_next_file,
+	more_kc_previous_file,
+	more_kc_run_editor
+} more_key_commands;
+struct number_command {
+	unsigned int number;
+	more_key_commands key;
+};
+
 struct more_control {
 	struct termios output_tty;	/* output terminal */
 	struct termios original_tty;	/* original terminal settings */
@@ -157,9 +191,8 @@ struct more_control {
 		long line_num;		/* line number */
 	} context,
 	  screen_start;
-	cc_t last_key_command;		/* previous more key command */
-	int last_key_arg;		/* previous key command argument */
-	int last_colon_command;		/* is a colon-prefixed key command */
+	unsigned int leading_number;	/* number in front of key command */
+	struct number_command previous_command;	/* previous key command */
 	char *shell_line;		/* line to execute in subshell */
 #ifdef HAVE_MAGIC
 	magic_t magic;			/* libmagic database entries */
@@ -177,14 +210,15 @@ struct more_control {
 		fold_long_lines:1,	/* fold long lines */
 		hard_tabs:1,		/* print spaces instead of '\t' */
 		hard_tty:1,		/* is this hard copy terminal (a printer or such) */
+		leading_colon:1,	/* key command has leading ':' character */
 		is_paused:1,		/* is output paused */
 		no_quit_dialog:1,	/* suppress quit dialog */
 		no_scroll:1,		/* do not scroll, clear the screen and then display text */
 		no_tty_in:1,		/* is input in interactive mode */
 		no_tty_out:1,		/* is output in interactive mode */
 		print_banner:1,		/* print file name banner */
+		reading_num:1,		/* are we reading leading_number */
 		report_errors:1,	/* is an error reported */
-		run_previous_command:1,	/* run previous key command */
 		search_at_start:1,	/* search pattern defined at start up */
 		search_called:1,	/* previous more command was a search */
 		squeeze_spaces:1,	/* suppress white space */
@@ -736,6 +770,12 @@ static cc_t read_user_input(struct more_control *ctl)
 	cc_t c;
 
 	errno = 0;
+	/*
+	 * Key commands can be read() from either stderr or stdin.  If they
+	 * are read from stdin such as 'cat file.txt | more' then the pipe
+	 * input is understood as series key commands - and that is not
+	 * wanted.  Keep the read() reading from stderr.
+	 */
 	if (read(STDERR_FILENO, &c, 1) <= 0) {
 		if (errno != EINTR)
 			more_exit(ctl);
@@ -745,27 +785,124 @@ static cc_t read_user_input(struct more_control *ctl)
 	return c;
 }
 
-/* Read a decimal number from the terminal.  Set cmd to the non-digit
+/* Read a number and command from the terminal.  Set cmd to the non-digit
  * which terminates the number. */
-static int read_number(struct more_control *ctl, cc_t *cmd)
+static struct number_command read_command(struct more_control *ctl)
 {
-	int i;
-	cc_t ch;
-
-	i = 0;
-	ch = ctl->output_tty.c_cc[VKILL];
-	for (;;) {
-		ch = read_user_input(ctl);
-		if (isdigit(ch))
-			i = i * 10 + ch - '0';
-		else if (ch == ctl->output_tty.c_cc[VKILL])
-			i = 0;
-		else {
-			*cmd = ch;
+	cc_t input[8] = { 0 };
+	ssize_t i, ilen;
+	struct number_command cmd = { .key = more_kc_unknown_command };
+
+	/* See stderr note in read_user_input() */
+	if ((ilen = read(STDERR_FILENO, &input, sizeof(input))) <= 0)
+		return cmd;
+	if (2 < ilen) {
+		if (!memcmp(input, ARROW_UP, sizeof(ARROW_UP))) {
+			cmd.key = more_kc_backwards;
+			return cmd;
+		} else if (!memcmp(input, ARROW_DOWN, sizeof(ARROW_DOWN))) {
+			cmd.key = more_kc_skip_forward;
+			return cmd;
+		} else if (!memcmp(input, PAGE_UP, sizeof(PAGE_UP))) {
+			cmd.key = more_kc_backwards;
+			return cmd;
+		} else if (!memcmp(input, PAGE_DOWN, sizeof(PAGE_DOWN))) {
+			cmd.key = more_kc_skip_forward;
+			return cmd;
+		}
+	}
+	for (i = 0; i < ilen; i++) {
+		if (isdigit(input[i])) {
+			if (0 < ctl->reading_num) {
+				ctl->leading_number *= 10;
+				ctl->leading_number += input[i] - '0';
+			} else
+				ctl->leading_number = input[i] - '0';
+			ctl->reading_num = 1;
+			continue;
+		}
+		cmd.number = ctl->leading_number;
+		ctl->reading_num = 0;
+		ctl->leading_number = 0;
+		if (ctl->leading_colon) {
+			ctl->leading_colon = 0;
+			switch (input[i]) {
+			case 'f':
+				cmd.key = more_kc_display_file_and_line;
+				return cmd;
+			case 'n':
+				cmd.key = more_kc_next_file;
+				return cmd;
+			case 'p':
+				cmd.key = more_kc_previous_file;
+				return cmd;
+			default:
+				cmd.key = more_kc_unknown_command;
+				return cmd;
+			}
+		}
+		/* command is a single char */
+		switch (input[i]) {
+		case '.':
+			cmd.key = more_kc_repeat_previous;
+			break;
+		case ':':
+			ctl->leading_colon = 1;
+			break;
+		case 'b':
+		case CTRL('B'):
+			cmd.key = more_kc_backwards;
+			break;
+		case ' ':
+			cmd.key = more_kc_jump_lines_per_screen;
+			break;
+		case 'z':
+			cmd.key = more_kc_set_lines_per_screen;
+			break;
+		case 'd':
+		case CTRL('D'):
+			cmd.key = more_kc_set_scroll_len;
+			break;
+		case 'q':
+		case 'Q':
+			cmd.key = more_kc_quit;
+			break;
+		case 'f':
+		case 's':
+		case CTRL('F'):
+			cmd.key = more_kc_skip_forward;
+			break;
+		case '\n':
+			cmd.key = more_kc_next_line;
+			break;
+		case '\f':
+			cmd.key = more_kc_clear_screen;
+			break;
+		case '\'':
+			cmd.key = more_kc_previous_search_match;
+			break;
+		case '=':
+			cmd.key = more_kc_display_line;
+			break;
+		case 'n':
+			cmd.key = more_kc_repeat_search;
+			break;
+		case '/':
+			cmd.key = more_kc_search;
+			break;
+		case '!':
+			cmd.key = more_kc_run_shell;
+			break;
+		case '?':
+		case 'h':
+			cmd.key = more_kc_help;
+			break;
+		case 'v':
+			cmd.key = more_kc_run_editor;
 			break;
 		}
 	}
-	return i;
+	return cmd;
 }
 
 /* Change displayed file from command line list to next nskip, where nskip
@@ -1128,7 +1265,7 @@ static void run_shell(struct more_control *ctl, char *filename)
 	erase_to_col(ctl, 0);
 	putchar('!');
 	fflush(NULL);
-	if (ctl->run_previous_command && ctl->shell_line)
+	if (ctl->previous_command.key == more_kc_run_shell && ctl->shell_line)
 		fputs(ctl->shell_line, stdout);
 	else {
 		ttyin(ctl, cmdbuf, sizeof(cmdbuf) - 2, '!');
@@ -1145,60 +1282,6 @@ static void run_shell(struct more_control *ctl, char *filename)
 	execute(ctl, filename, ctl->shell, ctl->shell, "-c", ctl->shell_line, 0);
 }
 
-/* Execute a colon-prefixed command.  Returns <0 if not a command that
- * should cause more of the file to be printed. */
-static int colon_command(struct more_control *ctl, char *filename, int cmd, int nlines)
-{
-	char ch;
-
-	if (cmd == 0)
-		ch = read_user_input(ctl);
-	else
-		ch = cmd;
-	ctl->last_colon_command = ch;
-	switch (ch) {
-	case 'f':
-		erase_to_col(ctl, 0);
-		if (!ctl->no_tty_in)
-			ctl->prompt_len =
-			    printf(_("\"%s\" line %d"), ctl->file_names[ctl->argv_position], ctl->current_line);
-		else
-			ctl->prompt_len = printf(_("[Not a file] line %d"), ctl->current_line);
-		fflush(NULL);
-		return -1;
-	case 'n':
-		if (nlines == 0) {
-			if (ctl->argv_position >= ctl->num_files - 1)
-				more_exit(ctl);
-			nlines++;
-		}
-		putchar('\r');
-		erase_to_col(ctl, 0);
-		change_file(ctl, nlines);
-		return 0;
-	case 'p':
-		if (ctl->no_tty_in) {
-			fprintf(stderr, "\a");
-			return -1;
-		}
-		putchar('\r');
-		erase_to_col(ctl, 0);
-		if (nlines == 0)
-			nlines++;
-		change_file(ctl, -nlines);
-		return 0;
-	case '!':
-		run_shell(ctl, filename);
-		return -1;
-	case 'q':
-	case 'Q':
-		more_exit(ctl);
-	default:
-		fprintf(stderr, "\a");
-		return -1;
-	}
-}
-
 /* Skip n lines in the file f */
 static void skip_lines(struct more_control *ctl)
 {
@@ -1503,12 +1586,11 @@ static int skip_forwards(struct more_control *ctl, int nlines, cc_t comchar)
  * display in the current file, zero is returned. */
 static int more_key_command(struct more_control *ctl, char *filename)
 {
-	int nlines;
 	int retval = 0;
-	cc_t colonch;
-	int done = 0;
-	cc_t comchar;
+	int done = 0, search_again = 0;
 	char cmdbuf[INIT_BUF];
+	struct number_command cmd;
+
 	if (!ctl->report_errors)
 		output_prompt(ctl, filename);
 	else
@@ -1517,72 +1599,51 @@ static int more_key_command(struct more_control *ctl, char *filename)
 	for (;;) {
 		if (more_poll(ctl, -1) != 0)
 			continue;
-		nlines = read_number(ctl, &comchar);
-		ctl->run_previous_command = colonch = 0;
-		if (comchar == '.') {	/* Repeat last command */
-			ctl->run_previous_command++;
-			comchar = ctl->last_key_command;
-			nlines = ctl->last_key_arg;
-			if (ctl->last_key_command == ':')
-				colonch = ctl->last_colon_command;
-		}
-		ctl->last_key_command = comchar;
-		ctl->last_key_arg = nlines;
-		if (comchar == ctl->output_tty.c_cc[VERASE]) {
-			erase_to_col(ctl, 0);
-			output_prompt(ctl, filename);
+		cmd = read_command(ctl);
+		if (cmd.key == more_kc_unknown_command)
 			continue;
-		}
-		switch (comchar) {
-		case ':':
-			retval = colon_command(ctl, filename, colonch, nlines);
-			if (retval >= 0)
-				done++;
-			break;
-		case 'b':
-		case CTRL('B'):
+		if (cmd.key == more_kc_repeat_previous)
+			cmd = ctl->previous_command;
+		switch (cmd.key) {
+		case more_kc_backwards:
 			if (ctl->no_tty_in) {
 				fprintf(stderr, "\a");
 				return -1;
 			}
-			retval = skip_backwards(ctl, nlines);
+			retval = skip_backwards(ctl, cmd.number);
 			done = 1;
 			break;
-		case ' ':
-		case 'z':
-			if (nlines == 0)
-				nlines = ctl->lines_per_screen;
-			else if (comchar == 'z')
-				ctl->lines_per_screen = nlines;
-			retval = nlines;
+		case more_kc_jump_lines_per_screen:
+		case more_kc_set_lines_per_screen:
+			if (cmd.number == 0)
+				cmd.number = ctl->lines_per_screen;
+			else if (cmd.key == more_kc_set_lines_per_screen)
+				ctl->lines_per_screen = cmd.number;
+			retval = cmd.number;
 			done = 1;
 			break;
-		case 'd':
-		case CTRL('D'):
-			if (nlines != 0)
-				ctl->d_scroll_len = nlines;
+		case more_kc_set_scroll_len:
+			if (cmd.number != 0)
+				ctl->d_scroll_len = cmd.number;
 			retval = ctl->d_scroll_len;
 			done = 1;
 			break;
-		case 'q':
-		case 'Q':
+		case more_kc_quit:
 			more_exit(ctl);
-		case 's':
-		case 'f':
-		case CTRL('F'):
-			if (skip_forwards(ctl, nlines, comchar))
+		case more_kc_skip_forward:
+			if (skip_forwards(ctl, cmd.number, cmd.number))
 				retval = ctl->lines_per_screen;
 			done = 1;
 			break;
-		case '\n':
-			if (nlines != 0)
-				ctl->lines_per_screen = nlines;
+		case more_kc_next_line:
+			if (cmd.number != 0)
+				ctl->lines_per_screen = cmd.number;
 			else
-				nlines = 1;
-			retval = nlines;
+				cmd.number = 1;
+			retval = cmd.number;
 			done = 1;
 			break;
-		case '\f':
+		case more_kc_clear_screen:
 			if (!ctl->no_tty_in) {
 				more_clear_screen(ctl);
 				more_fseek(ctl, ctl->screen_start.row_num);
@@ -1594,7 +1655,7 @@ static int more_key_command(struct more_control *ctl, char *filename)
 				fprintf(stderr, "\a");
 				break;
 			}
-		case '\'':
+		case more_kc_previous_search_match:
 			if (!ctl->no_tty_in) {
 				erase_to_col(ctl, 0);
 				fputs(_("\n***Back***\n\n"), stdout);
@@ -1607,49 +1668,82 @@ static int more_key_command(struct more_control *ctl, char *filename)
 				fprintf(stderr, "\a");
 				break;
 			}
-		case '=':
+		case more_kc_display_line:
 			erase_to_col(ctl, 0);
 			ctl->prompt_len = printf("%d", ctl->current_line);
 			fflush(NULL);
 			break;
-		case 'n':
+		case more_kc_display_file_and_line:
+			erase_to_col(ctl, 0);
+			if (!ctl->no_tty_in)
+				ctl->prompt_len =
+				    printf(_("\"%s\" line %d"),
+				           ctl->file_names[ctl->argv_position], ctl->current_line);
+			else
+				ctl->prompt_len = printf(_("[Not a file] line %d"),
+							 ctl->current_line);
+			fflush(NULL);
+			break;
+		case more_kc_repeat_search:
 			if (!ctl->previous_search) {
 				more_error(ctl, _("No previous regular expression"));
 				break;
 			}
-			ctl->run_previous_command = 1;
+			search_again = 1;
 			/* fallthrough */
-		case '/':
-			if (nlines == 0)
-				nlines++;
+		case more_kc_search:
+			if (cmd.number == 0)
+				cmd.number++;
 			erase_to_col(ctl, 0);
 			putchar('/');
 			ctl->prompt_len = 1;
 			fflush(NULL);
-			if (ctl->run_previous_command) {
+			if (search_again) {
 				fputc('\r', stderr);
-				search(ctl, ctl->previous_search, nlines);
+				search(ctl, ctl->previous_search, cmd.number);
+				search_again = 0;
 			} else {
 				ttyin(ctl, cmdbuf, sizeof(cmdbuf) - 2, '/');
 				fputc('\r', stderr);
 				ctl->next_search = xstrdup(cmdbuf);
-				search(ctl, ctl->next_search, nlines);
+				search(ctl, ctl->next_search, cmd.number);
 			}
 			retval = ctl->lines_per_screen - 1;
 			done = 1;
 			break;
-		case '!':
+		case more_kc_run_shell:
 			run_shell(ctl, filename);
 			break;
-		case '?':
-		case 'h':
+		case more_kc_help:
 			if (ctl->no_scroll)
 				more_clear_screen(ctl);
 			erase_to_col(ctl, 0);
 			runtime_usage();
 			output_prompt(ctl, filename);
 			break;
-		case 'v':	/* This case should go right before default */
+		case more_kc_next_file:
+			putchar('\r');
+			erase_to_col(ctl, 0);
+			if (cmd.number == 0)
+				cmd.number = 1;
+			if (ctl->argv_position + cmd.number >= (unsigned int)ctl->num_files)
+				more_exit(ctl);
+			change_file(ctl, cmd.number);
+			done = 1;
+			break;
+		case more_kc_previous_file:
+			if (ctl->no_tty_in) {
+				fprintf(stderr, "\a");
+				break;
+			}
+			putchar('\r');
+			erase_to_col(ctl, 0);
+			if (cmd.number == 0)
+				cmd.number = 1;
+			change_file(ctl, -cmd.number);
+			done = 1;
+			break;
+		case more_kc_run_editor:	/* This case should go right before default */
 			if (!ctl->no_tty_in) {
 				execute_editor(ctl, cmdbuf, filename);
 				break;
@@ -1670,8 +1764,11 @@ static int more_key_command(struct more_control *ctl, char *filename)
 			fflush(NULL);
 			break;
 		}
-		if (done)
+		ctl->previous_command = cmd;
+		if (done) {
+			cmd.key = more_kc_unknown_command;
 			break;
+		}
 	}
 	putchar('\r');
 	ctl->no_quit_dialog = 1;
-- 
2.26.2

Re: [PATCH 0/2] more(1) use libmagic and make page up/down and arrow keys work

[Karel Zak]

On Wed, May 06, 2020 at 08:19:21PM +0100, Sami Kerola wrote:
> Here are two changes to more(1) that where requested in earlier github
> pull[1] request to get review in maillist, so here we go.

Applied, thanks.

> First one of the changes will link more(1) with libmagic so that various
> binary formats can be detected with effect the pager will not mess up
> termial.

It seems we do not have devel package for libmagic in Fedora.

> Second change is perhaps a little bit more exciting.  It will add page-up,
> page-down, arrow-up, and arrow-down movements to more(1).  This pager has
> known now to scroll back and forth since long time ago, but most of the
> people do not seem to know key bindings so lets help users by assigning the
> most obvious buttons the way one might expect them to work. 

This is nice feature, maybe also add arrow keys to move one line
up/down.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 0/2] more(1) use libmagic and make page up/down and arrow keys work

[Sami Kerola]

On Tue, 12 May 2020 at 09:54, Karel Zak <kzak@redhat.com> wrote:
> On Wed, May 06, 2020 at 08:19:21PM +0100, Sami Kerola wrote:
> > Here are two changes to more(1) that where requested in earlier github
> > pull[1] request to get review in maillist, so here we go.
>
> Applied, thanks.
>
> > First one of the changes will link more(1) with libmagic so that various
> > binary formats can be detected with effect the pager will not mess up
> > termial.
>
> It seems we do not have devel package for libmagic in Fedora.

Oh, that's a little disappointing. Archlinux has libmagic in core
repository and that lead me thinking it is quite likely part of even
most minimal installation, but apparently I was wrong.

> > Second change is perhaps a little bit more exciting.  It will add page-up,
> > page-down, arrow-up, and arrow-down movements to more(1).  This pager has
> > known now to scroll back and forth since long time ago, but most of the
> > people do not seem to know key bindings so lets help users by assigning the
> > most obvious buttons the way one might expect them to work.
>
> This is nice feature, maybe also add arrow keys to move one line
> up/down.

Funny you said that. The more(1) movements are still quite disappointing
when trying to read the code. Sometime before Christmas I will try to
address that along with adding posix required more(1) key-bindings.
But there is no hope these would be ready before next util-linux release
so now we have a little unexpected movements for a bit. I hope that
is still an improvement to earlier situation when going backwards
seemed to be impossible for most of the users.

Thank you for review and merge Karel.

-- 
Sami Kerola
http://www.iki.fi/kerolasa/