[PATCH] less: fix display of line numbers

[walter harms]

Am 18.07.2015 11:06, schrieb Ron Yorston:
> Line numbers are displayed incorrectly on lines that have a search
> pattern highlighted.  The problem can be fixed by moving the call to
> lineno_str in print_found above the while loop that alters the value
> of the line pointer.  However, a more substantial rewrite results in
> savings.
> 
> function                                             old     new   delta
> buffer_print                                         688     697      +9
> .rodata                                           156077  156045     -32
> lineno_str                                            85       -     -85
> ------------------------------------------------------------------------------
> (add/remove: 0/1 grow/shrink: 1/1 up/down: 9/-117)           Total: -108 bytes
> 
> Signed-off-by: Ron Yorston <rmy at pobox.com>
> ---
>  miscutils/less.c | 39 +++++++++++++++------------------------
>  1 file changed, 15 insertions(+), 24 deletions(-)
> 
> diff --git a/miscutils/less.c b/miscutils/less.c
> index d5a68d1..10d2104 100644
> --- a/miscutils/less.c
> +++ b/miscutils/less.c
> @@ -665,27 +665,24 @@ static const char ctrlconv[] ALIGN1 =
>  	"\x40\x41\x42\x43\x44\x45\x46\x47\x48\x49\x40\x4b\x4c\x4d\x4e\x4f"
>  	"\x50\x51\x52\x53\x54\x55\x56\x57\x58\x59\x5a\x5b\x5c\x5d\x5e\x5f";
>  
> -static void lineno_str(char *nbuf9, const char *line)
> +static void print_lineno(const char *line)
>  {
> -	nbuf9[0] = '\0';
> -	if (option_mask32 & FLAG_N) {
> -		const char *fmt;
> -		unsigned n;
> -
> -		if (line == empty_line_marker) {
> -			memset(nbuf9, ' ', 8);
> -			nbuf9[8] = '\0';
> -			return;
> -		}
> +	const char *fmt;
> +	unsigned n = n; /* for compiler */
> +

you can remove the else if you turn the compare on its head.

fmt = "        ";
if ( line != empty_line_marker)

may be this helps so save a few bytes.

> +	if (line == empty_line_marker) {
> +		fmt = "        ";
> +	}
> +	else {
>  		/* Width of 7 preserves tab spacing in the text */
>  		fmt = "%7u ";
>  		n = LINENO(line) + 1;
> -		if (n > 9999999) {
> +		if (n > 9999999 && MAXLINES > 9999999) {
>  			n %= 10000000;
>  			fmt = "%07u ";
>  		}

could you explain in 1 line why the MAXLINES test is needed ?


> -		sprintf(nbuf9, fmt, n);
>  	}
> +	printf(fmt, n);
>  }
>  
>  
> @@ -698,7 +695,6 @@ static void print_found(const char *line)
>  	regmatch_t match_structs;
>  
>  	char buf[width];
> -	char nbuf9[9];
>  	const char *str = line;
>  	char *p = buf;
>  	size_t n;
> @@ -748,12 +744,7 @@ static void print_found(const char *line)
>  			match_status = 1;
>  	}
>  
> -	lineno_str(nbuf9, line);
> -	if (!growline) {
> -		printf(CLEAR_2_EOL"%s%s\n", nbuf9, str);
> -		return;
> -	}
> -	printf(CLEAR_2_EOL"%s%s%s\n", nbuf9, growline, str);
> +	printf("%s%s\n", growline ? growline : "", str);
>  	free(growline);
>  }
>  #else
> @@ -763,13 +754,9 @@ void print_found(const char *line);
>  static void print_ascii(const char *str)
>  {
>  	char buf[width];
> -	char nbuf9[9];
>  	char *p;
>  	size_t n;
>  
> -	lineno_str(nbuf9, str);
> -	printf(CLEAR_2_EOL"%s", nbuf9);
> -
>  	while (*str) {
>  		n = strcspn(str, controls);
>  		if (n) {
> @@ -803,6 +790,10 @@ static void buffer_print(void)
>  
>  	move_cursor(0, 0);
>  	for (i = 0; i <= max_displayed_line; i++) {
> +		printf(CLEAR_2_EOL);
> +		if (option_mask32 & FLAG_N) {
> +			print_lineno(buffer[i]);
> +		}

i guess you can remove the {}

>  		if (pattern_valid)
>  			print_found(buffer[i]);
>  		else

just my 2 cents,
re
 wh