/hg/icedtea-web: IcedTea-Web Splashscreen allows zero or one hyp...

[Jiri Vanek]

On 10/06/2014 05:10 PM, Lukasz Dracz wrote:
> Hello,
>
> Sorry, I thought at the time the change/difference was small enough but I agree even the smallest changes should get reviewed. The reasoning behind the change is that I thought -*headless  was not a correct change, since really only headless and -headless are accepted as options, anything with more dashes is not accepted such as --headless, ---headless, -----headless etc. It did not make sense to have a user put ---headless, then have the icedtea-web splash screen not appear only to get a message back that ---headless is not an accepted option. That is why I changed what was agreed upon to what is actually supported, which is only headless or -headless.
> Once again I am sorry, I did not mean to violate the review process. Do you want to support -*headless ? So anything from headless to --*--headless I can make the required changes if that is what is wanted in option parser. Should the change be reverted ?
>

In case that your parser supports only -headless or headles, tehn your chnage will be ok. I also 
like your ^ and [] enhancement. For that I will do nothing more then ping you adn verify stuff.

But I would swear, that we agreed  on -* support.

However in code I now can see the 	

  private boolean stringEqualsOption(String input, OptionsDefinitions.OPTIONS opt) {
         String option = opt.option.replaceAll("^-","").split("=")[0];
         input = input.replaceAll("^-","").split("=")[0];
         if (input.equals(option)) {
             return true;
         }
         return false;
     }


Which really accept only -{0,1}

And the method is not tested;)


So with that, you are cleared for this well intentioned  change, but please, try to avoid it. 
especially when original review is gone;)

Also please [rfc] attached patch, which is fixing both those issues.


Sorry for the "attack", but at least it clarified the  -/--/---// hyphens issue


J.

ps: * but please, really try to avoid such last-moment changes *
>
> ----- Original Message -----
>> From: "Jiri Vanek" <jvanek at redhat.com>
>> To: ldracz at icedtea.classpath.org, distro-pkg-dev at openjdk.java.net
>> Sent: Monday, October 6, 2014 4:30:33 AM
>> Subject: Re: /hg/icedtea-web: IcedTea-Web Splashscreen allows zero or one	hyp...
>>
>> Shouldnt it be zero to infinity hyphens?
>>> +++ b/launcher/launchers.in	Fri Oct 03 14:20:40 2014 -0400
>>> @@ -53,7 +53,7 @@
>>>        *)
>>>          ARGS[$j]="$1"
>>>          j=$((j+1))
>>> -      if [ "$1" = "-headless" ] ; then
>>> +      if [[ "$1" =~ ^[-]{0,1}headless ]] ; then
>>
>>    =>  +      if [[ "$1" =~ ^[-]*headless ]] ; then
>>
>> I rember it like this....
>>
>> ?
>>>            SPLASH="false"
>>>          fi
>>>          ;;
>>>
>>
>>
>> diff -r 6d62f68fb037 launcher/launchers.in
>> --- a/launcher/launchers.in	Mon Sep 22 17:10:07 2014 +0200
>> +++ b/launcher/launchers.in	Thu Sep 25 17:26:49 2014 -0400
>> @@ -53,7 +53,7 @@
>>        *)
>>          ARGS[$j]="$1"
>>          j=$((j+1))
>> -      if [ "$1" = "-headless" ] ; then
>> +      if [[ "$1" =~ -*headless ]] ; then
>>            SPLASH="false"
>>          fi
>>          ;;
>>
>>
>> is your original chnageset. I have not found discussion about this chnage,
>> and unless I overlooked it, it is violating the review process.
>>
>>
>> ????
>> J.
>>

-------------- next part --------------
A non-text attachment was scrubbed...
Name: multipleHyphensFix.patch
Type: text/x-patch
Size: 2944 bytes
Desc: not available
URL: <http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20141006/f2877fa3/multipleHyphensFix-0001.patch>