libtool-patches
[Top][All Lists]
Advanced

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

Re: [PATCH] Add an XSI replacement for func_split_short_opt.


From: Gary V. Vaughan
Subject: Re: [PATCH] Add an XSI replacement for func_split_short_opt.
Date: Mon, 28 Jun 2010 06:30:23 +0700

Hallo Ralf,

Thanks for the review.

On 27 Jun 2010, at 19:02, Ralf Wildenhues wrote:
> * Gary V. Vaughan wrote on Sun, Jun 27, 2010 at 12:58:03PM CEST:
>> Okay to push?
> 
> No, ${parameter:offset} and ${parameter:offset:length} are bash specific
> not XSI mandated.

Ah, okay.  Didn't think to look that up.  Nice catch.

> IIRC we already have a separate test for bash
> features that won't regress on non-bash XSI shells.  Please add the test
> there.

I guess you were referring to the += feature test.  I decided instead
to make a separate new ${foo:n:m} test alongside in case we ever encounter
a shell that supports one but not the other.

> Then, the patch is ok if you have ensured that our testsuite
> covers the code path (i.e., tries a short option that needs splitting
> this way) or added it there,

I hadn't checked the short option splitting, but during the process of
writing the test uncovered (and fixed) a small bug.  Good call.  Thanks.

> help.at would be a good place.

Instead, I created a new getopt-m4sh.at file, since I should really get
around to writing some unit tests for the other features of getopt.m4sh
as well as just the option splitting.  I'll add them to this new file
with additional patches later on.

> And tested
> on one system that has a good shell and one that has a bad shell.

Done.  Passed.  And pushed!

>> * libltdl/m4/libtool.m4 (_LT_CHECK_SHELL_FEATURES): Also ensure
>> that XSI substring extraction is supported.
>> (_LT_PROG_XSI_SHELLFNS): Use XSI substring extraction to split
>> short options to avoid unnecessary forks if the host shell
>> supports that.

Cheers,
-- 
Gary V. Vaughan (address@hidden)



reply via email to

[Prev in Thread] Current Thread [Next in Thread]