Re: [PATCH] generate-cmdlist: re-implement as shell script

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Aug 20, 2015, 18:11 UTC
Message-ID: <CAPig+cROaXkRVbpRJYiAL1yiXB1HV_12To=VMO8kZVk_AFE+2w@mail.gmail.com>
In-Reply-To: <xmqqegiy9a4d.fsf@gitster.dls.corp.google.com>

On Wed, Aug 19, 2015 at 6:38 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 8 quoted lines

> Eric Sunshine <sunshine@sunshineco.com> writes:
>> In addition to reviving 527ec39^:generate-cmdlist.sh and extending it, I
>> also re-indented it with tabs instead of spaces, so it's helpful to
>> ignore whitespace changes when comparing the old and new versions of the
>> shell script.
>
> Hmph.  Perhaps we can view it as part of reverting 527ec39 and then
> redoing it in shell.

I'm having trouble understanding. Are you asking that this be presented as a series of patches which first revert 527ec39, then do the whitespace cleanup, and then augment the script for the extended functionality? If so, it's a bit problematic because the original script still expects the 'common' tag to be present, but that tag was removed by the subsequent patch 2f5b495 (command-list.txt: drop the "common" tag, 2015-05-21). So, reverting 527ec39 would also require reverting 2f5b495.

Show 5 quoted lines

> The way the shell script accumulates matchgrp
> variable (i.e. the literal LF in ${var:+string}) makes me feel some
> possible portability scare, which might be solved in a more stupid
> (i.e. not giving various reimplementations of Bourne shells a chance
> to screw it up) way by using another temporary file[...]

In addition to the literal newline and the temporary file, other options I considered included assigning the newline to a variable and then interpolating that variable (${var:+$LF}), or expanding the list of patterns into a set of '-e' arguments for grep. But, I think I'll just go with the temporary file.