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

[Nguyen Thai Ngoc Duy <pclouds@gmail.com>]

On 3/9/10, René Scharfe <rene.scharfe@lsrfire.ath.cx> wrote:
> 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.

>  >  - 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.

>  >  - 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];

>  >>  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