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
