Re: [PATCH 0/7] grep.c: teach --column to 'git-grep(1)'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jun 19, 2018 at 01:48:47PM -0400, Jeff King wrote:
> On Tue, Jun 19, 2018 at 07:33:39PM +0200, René Scharfe wrote:
> > Disabling that optimization for --column wouldn't be a regression since
> > it's a new option..  Picking a random result (based on the order of
> > evaluation) seems sloppy and is probably going to surprise users.
>
> I don't see it as a random result; short-circuiting logic is well
> understood and we follow the user's ordering.

Agreed. I would prefer _not_ to apply the patch that I sent to René,
since I think it adds more complexity than its worth (precisely because
the short-circuiting logic is known, though certainly the problem gets
worse as the expression tree grows).

> I think the place where it's _most_ ugly is "--column --color", where we
> may color the short-circuited value in the second pass.

Agreed again, but it's worth noting that --color is the default. To play
devil's advocate, the use-case that I imagine will be most common is
with "git jump," so perhaps that doesn't matter as much.

> > We could add an optimizer pass to reduce the number of regular
> > expressions in certain cases if that is really too slow.  E.g. this:
>
> Yes, we actually discussed this kind of transformation. I think it's way
> out of scope for this patch series, though. If we do anything more, I
> think it should be to disable short-circuiting when --column is in use.

Piggy-backing on what I said to René, agreed again.

> -Peff

Thanks,
Taylor



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux