Re: [PATCH 00/12] Support columinized output in tag/branch/ls-files/grep

From: Nguyen Thai Ngoc Duy <pclouds@gmail.com>
Date: Mar 10, 2010, 00:27 UTC
Message-ID: <fcaeb9bf1003091627p65ad6e60u4bbae2eb4e859f13@mail.gmail.com>
In-Reply-To: <4B967C36.90309@lsrfire.ath.cx>

On 3/9/10, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:

Show 17 quoted lines

> Am 08.03.2010 15:32, schrieb Nguyen Thai Ngoc Duy:
>
> > On 3/8/10, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
>
> >>  I'm not so sure about the interface, though.  Does the task really
>  >>  warrant adding a new git command?
>  >
>  > Several reasons:
>  >  - Can be reused outside of git (I was surprised Solaris did not have "column")
>
>
> Granted, we have a precedent: git stripspace.  But I don't like that
>  one, either. ;)  If it's not specific to git, then it should not be a
>  git command.
>
>  Perhaps name it git-column--helper, so that it can still be used by git
>  commands that are implemented as scripts?

OK.

>
>  >  - Easier to test
>
>  Name it test-column?

If it turns out no external process is needed for column layout.

Show 5 quoted lines

>  >  - Minimum code change in modifed commands
>
>
> OK, forking out is easy, but replacing printf() calls with calls to feed
>  the columnizer shouldn't be _that_ intrusive, either.

Well, also fwrite() and write(). If disliked "if (blah) feed_it(); else printf(as normal);" construct. But we can wrap it to feed_or_printf(). We may also need to buffer printf() results until a full line is received. Let me try and see how intrusive it is.

Show 11 quoted lines

>  >  - I can play with more complicated column layout, with minimum code
>  > change in git (ok, that's the third reason).
>
>
> This is possible regardless of the way how the columnizer is invoked if
>  its full functionality is exposed through the command line parameter.
>
>  With "more complicated", do you perhaps mean what GNU ls does, namely
>  having non-uniform column widths?  I never consciously noticed that it
>  actually goes out of its way to cram as may columns on the screen as
>  possible, it just feels so natural. :)

That. And aligned grep output like this

pclouds@do ~/w/git/builtin $ git grep -n 38 count-objects.c | 35 | if (cp - ent->d_name != 38) count-objects.c | 39 | memcpy(path + len + 3, ent->d_name, 38); count-objects.c | 59 | memcpy(hex+2, ent->d_name, 38); fsck.c | 405 | if (strlen(de->d_name) == 38) { gc.c | 112 | if (strspn(ent->d_name, "0123456789abcdef") != 38 || gc.c | 113 | ent->d_name[38] != '\0') prune-packed.c | 24 | if (strlen(de->d_name) != 38) prune-packed.c | 26 | memcpy(hex+2, de->d_name, 38); prune-packed.c | 31 | memcpy(pathname + len, de->d_name, 38); prune.c | 64 | if (strlen(de->d_name) == 38) { receive-pack.c | 588 | char hdr_arg[38]; upload-archive.c | 86 | char buf[16384];

Show 10 quoted lines

>  >>  If a --column parameter is added, I think it should expose the full
>  >>  range of options, i.e. fill columns first (ls -C style), fill rows first
>  >>  (ls -x style) as well as off (ls -1 style) and auto.
>  >
>  > Maybe an env variable would be better, so you can pass abitrary
>  > arguments to git-column. "--column=auto" should be supported, of
>  > course.
>
>
> I don't see any benefit of an environment variable over config options.

Currently we may pass --column=<foo> from a porcelain to "git column
--mode=<foo>", <foo> could be column first, or row first, or either
with non-uniform columns (not implemented yet). We can also pass other
things to "git column". Putting everything in "<foo>" is OK, although
looks ugly. In my private tree, I also have "git column
--min-rows/--max-items" that forces the columnizer to one column mode
if:
 - there will be only one or two rows after columnized, too wide
screen for example (--min-rows)
 - too many lines and the layout has not been fixed, so nothing gets
printed (--max-items). Forcing back to one column mode to stop wait
time.

-- 
Duy