Skip site navigation (1)Skip section navigation (2)
Date:      Tue, 5 Mar 2002 20:57:33 +1100
From:      "Tim J. Robbins" <tim@robbins.dropbear.id.au>
To:        Kyle Martin <mkm@ieee.org>
Cc:        freebsd-standards@FreeBSD.ORG
Subject:   Re: ls(1) patch for review
Message-ID:  <20020305205733.A31150@descent.robbins.dropbear.id.au>
In-Reply-To: <20020305030632.A1990@marvin.idsi.net>; from mkm@ieee.org on Tue, Mar 05, 2002 at 03:06:32AM -0600
References:  <20020305030632.A1990@marvin.idsi.net>

next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, Mar 05, 2002 at 03:06:32AM -0600, Kyle Martin wrote:

> +.It Fl p
> +Write a slash
> +.Pq Pa \&\
> +after each filename if that file is a directory.

It writes a /, and the \ confuses groff. How about this:
.It Fl p
Write a slash
.Pq Ql /
after each filename if that file is a directory.

> +.It Fl x
> +The same as -C, except that the multi-text-column output is produced with entries sorted across, rather tahn down, the columns.

This line should be wrapped at 80 columns or less.

>  void
> +printstream(DISPLAY *dp)
> +{
> +	FTSENT *p;
> +
> +	for (p = dp->list; p; p = p->fts_link) {
> +		if (p->fts_number != NO_PRINT) {
> +			(void)printaname(p, dp->s_inode, dp->s_block);
> +			if (p->fts_link)
> +				(void)printf(", ");
> +		}
> +	}
> +	(void)putchar('\n');
> +}

The lines output by the stream format must not exceed the width of
the terminal. See GNU ls for an example.

> +		
> +void
>  printcol(DISPLAY *dp)
>  {
>  	extern int termwidth;
>  	static FTSENT **array;
>  	static int lastentries = -1;
> @@ -280,25 +295,47 @@
>  	if (num % numcols)
>  		++numrows;
>  
>  	if (dp->list->fts_level != FTS_ROOTLEVEL && (f_longform || f_size))
>  		(void)printf("total %lu\n", howmany(dp->btotal, blocksize));
> -	for (row = 0; row < numrows; ++row) {
> -		endcol = colwidth;
> -		for (base = row, chcnt = col = 0; col < numcols; ++col) {
> -			chcnt += printaname(array[base], dp->s_inode,
> -			    dp->s_block);
> -			if ((base += numrows) >= num)
> -				break;
> -			while ((cnt = ((chcnt + tabwidth) & ~(tabwidth - 1)))
> -			    <= endcol) {
> -				(void)putchar(f_notabs ? ' ' : '\t');
> -				chcnt = cnt;
> +
> +	if (f_sortacross) {

*snip*

Wouldn't it be better to have a separate function for each of these two
output functions instead of having one big `if' and almost nothing
in common between the two?

I think there is also a problem with -x outputting tabs before newlines
(\t\n) but I can't quite track it down. Different terminals handle this
differently, it makes the output ugly on some.

>   * print [inode] [size] name
> @@ -362,33 +399,44 @@
>  }
>  
>  static int
>  printtype(u_int mode)
>  {
> -	switch (mode & S_IFMT) {
> -	case S_IFDIR:
> -		(void)putchar('/');
> -		return (1);
> -	case S_IFIFO:
> -		(void)putchar('|');
> -		return (1);
> -	case S_IFLNK:
> -		(void)putchar('@');
> -		return (1);
> -	case S_IFSOCK:
> -		(void)putchar('=');
> -		return (1);
> -	case S_IFWHT:
> -		(void)putchar('%');
> -		return (1);
> -	default:
> -	}
> -	if (mode & (S_IXUSR | S_IXGRP | S_IXOTH)) {
> -		(void)putchar('*');
> -		return (1);
> +	if (f_slash) {
> +		if ((mode & S_IFMT) == S_IFDIR) {
> +			(void)putchar('/');
> +			return (1);
> +		}
> +		else
> +			return (0);
> +	}
> +
> +	else {

else needs to go on the line with the closing brace.

> +			switch (mode & S_IFMT) {
> +			case S_IFDIR:
> +				(void)putchar('/');
> +				return (1);
> +			case S_IFIFO:
> +				(void)putchar('|');
> +				return (1);
> +			case S_IFLNK:
> +				(void)putchar('@');
> +				return (1);
> +			case S_IFSOCK:
> +				(void)putchar('=');
> +				return (1);
> +			case S_IFWHT:
> +				(void)putchar('%');
> +				return (1);
> +			default:

This default: is not needed and is possibily illegal. This wasn't added
by your change, but should be fixed:
"warning: ANSI C forbids label at end of compound statement"

Wouldn't it be better to have the -p and -F functionality split off
into separate functions?

Keep up the good work.


Tim

To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-standards" in the body of the message




Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020305205733.A31150>