Aw: Re: [PATCH] Fix hardcoded strings in pppsetup.cgi

Message ID op.xzmb0movcahio0@honk.fritz.box
State Accepted
Delegated to: Michael Tremer
Headers

Message

Lars Schuhmacher June 3, 2015, 5:49 a.m. UTC
  Some strings for PPTP were hardcoded instead of using i18n features.

Signed-off-by: Lars Schuhmacher <larsen007@web.de>
---
  html/cgi-bin/pppsetup.cgi | 6 +++---
  langs/de/cgi-bin/de.pl    | 3 +++
  langs/en/cgi-bin/en.pl    | 3 +++
  3 files changed, 9 insertions(+), 3 deletions(-)
  

Comments

Michael Tremer June 3, 2015, 6:01 a.m. UTC | #1
Hi,

this patch looks okay, but it won't apply. Git error:

Applying: Aw: Re: [PATCH] Fix hardcoded strings in pppsetup.cgi
error: patch failed: html/cgi-bin/pppsetup.cgi:793
error: html/cgi-bin/pppsetup.cgi: patch does not apply
error: patch failed: langs/de/cgi-bin/de.pl:1822
error: langs/de/cgi-bin/de.pl: patch does not apply
error: patch failed: langs/en/cgi-bin/en.pl:1852
error: langs/en/cgi-bin/en.pl: patch does not apply
Patch failed at 0001 Aw: Re: [PATCH] Fix hardcoded strings in pppsetup.cgi

This is probably caused by copy-and-paste and replacing tabs by spaces
or similar.

-Michael

On Tue, 2015-06-02 at 21:49 +0200, Larsen wrote:
> Some strings for PPTP were hardcoded instead of using i18n features.
> 
> Signed-off-by: Lars Schuhmacher <larsen007@web.de>
> ---
>   html/cgi-bin/pppsetup.cgi | 6 +++---
>   langs/de/cgi-bin/de.pl    | 3 +++
>   langs/en/cgi-bin/en.pl    | 3 +++
>   3 files changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/html/cgi-bin/pppsetup.cgi b/html/cgi-bin/pppsetup.cgi
> index 33f521e..59677a4 100644
> --- a/html/cgi-bin/pppsetup.cgi
> +++ b/html/cgi-bin/pppsetup.cgi
> @@ -793,15 +793,15 @@ print <<END
>           <td colspan='4' width='100%' bgcolor='$color{'color20'}'><b>$Lang::tr{'pptp settings'}</b></td>
>   </tr>
>   <tr>
> -        <td width='25%'>Peer</td>
> +        <td width='25%'>$Lang::tr{'pptp peer'}:</td>
>           <td colspan='3'><input size=50 type='text' name='PPTP_PEER' value='$pppsettings{'PPTP_PEER'}' /></td>
>   </tr>
>   <tr>
> -        <td width='25%'>My Netconfig</td>
> +        <td width='25%'>$Lang::tr{'pptp netconfig'}:</td>
>           <td colspan='3'><input size=50 type='text' name='PPTP_NICCFG' value='$pppsettings{'PPTP_NICCFG'}' /></td>
>   </tr>
>   <tr>
> -        <td width='25%'>PPTP Route&nbsp;<img src='/blob.gif' alt='*' /></td>
> +        <td width='25%'>$Lang::tr{'pptp route'}:&nbsp;<img src='/blob.gif' alt='*' /></td>
>           <td colspan='3'><input size=50 type='text' name='PPTP_ROUTE' value='$pppsettings{'PPTP_ROUTE'}' /></td>
>   </tr>
> 
> diff --git a/langs/de/cgi-bin/de.pl b/langs/de/cgi-bin/de.pl
> index 8adefdf..65277a9 100644
> --- a/langs/de/cgi-bin/de.pl
> +++ b/langs/de/cgi-bin/de.pl
> @@ -1822,6 +1822,9 @@
>   'ppp setup' => 'PPP-Einstellungen',
>   'pppoe' => 'PPPoE',
>   'pppoe settings' => 'Zusätzliche PPPoE-Einstellungen:',
> +'pptp netconfig' => 'Eigene Netzkonfiguration',
> +'pptp peer' => 'Gegenstelle',
> +'pptp route' => 'PPTP Route',
>   'pptp settings' => 'Zusätzliche PPTP-Einstellungen:',
>   'pre-shared key is too short' => 'Pre-shared Schlüsel ist zu kurz',
>   'prefered master' => 'Prefered Master',
> diff --git a/langs/en/cgi-bin/en.pl b/langs/en/cgi-bin/en.pl
> index 43601c8..3e60b00 100644
> --- a/langs/en/cgi-bin/en.pl
> +++ b/langs/en/cgi-bin/en.pl
> @@ -1852,6 +1852,9 @@
>   'ppp setup' => 'PPP setup',
>   'pppoe' => 'PPPoE',
>   'pppoe settings' => 'Additional PPPoE settings:',
> +'pptp netconfig' => 'My Netconfig',
> +'pptp peer' => 'Peer',
> +'pptp route' => 'PPTP Route',
>   'pptp settings' => 'Additional PPTP settings:',
>   'pre-shared key is too short' => 'Pre-shared key is too short.',
>   'prefered master' => 'Prefered Master',
  
Lars Schuhmacher June 3, 2015, 7:05 a.m. UTC | #2
On Tue, 02 Jun 2015 22:01:18 +0200, Michael Tremer <michael.tremer@ipfire.org> wrote:

> Hi,
>
> this patch looks okay, but it won't apply. Git error:
>
> Applying: Aw: Re: [PATCH] Fix hardcoded strings in pppsetup.cgi
> error: patch failed: html/cgi-bin/pppsetup.cgi:793
> error: html/cgi-bin/pppsetup.cgi: patch does not apply
> error: patch failed: langs/de/cgi-bin/de.pl:1822
> error: langs/de/cgi-bin/de.pl: patch does not apply
> error: patch failed: langs/en/cgi-bin/en.pl:1852
> error: langs/en/cgi-bin/en.pl: patch does not apply
> Patch failed at 0001 Aw: Re: [PATCH] Fix hardcoded strings in pppsetup.cgi
>
> This is probably caused by copy-and-paste and replacing tabs by spaces
> or similar.
  
Lars Schuhmacher June 3, 2015, 7:07 a.m. UTC | #3
On Tue, 02 Jun 2015 23:05:10 +0200, Larsen <larsen007@web.de> wrote:

> On Tue, 02 Jun 2015 22:01:18 +0200, Michael Tremer <michael.tremer@ipfire.org> wrote:
>
>> Hi,
>>
>> this patch looks okay, but it won't apply. Git error:
>>
>> Applying: Aw: Re: [PATCH] Fix hardcoded strings in pppsetup.cgi
>> error: patch failed: html/cgi-bin/pppsetup.cgi:793
>> error: html/cgi-bin/pppsetup.cgi: patch does not apply
>> error: patch failed: langs/de/cgi-bin/de.pl:1822
>> error: langs/de/cgi-bin/de.pl: patch does not apply
>> error: patch failed: langs/en/cgi-bin/en.pl:1852
>> error: langs/en/cgi-bin/en.pl: patch does not apply
>> Patch failed at 0001 Aw: Re: [PATCH] Fix hardcoded strings in pppsetup.cgi
>>
>> This is probably caused by copy-and-paste and replacing tabs by spaces
>> or similar.
>

(now again without hitting Ctrl+Enter too early... damn)

I saved the patch from your last reply and compared it to the original patch file
that was created by TortoiseGit. There were no differences.

I also patched pppsetup.cgi in my VM with Core 90 (didn´t try for the lang files)
and this also went fine.

There are no tabs in the patch.

I don´t know what could be wrong with it... =(
  
Michael Tremer June 5, 2015, 2:23 a.m. UTC | #4
On Tue, 2015-06-02 at 23:07 +0200, Larsen wrote:
> On Tue, 02 Jun 2015 23:05:10 +0200, Larsen <larsen007@web.de> wrote:
> 
> > On Tue, 02 Jun 2015 22:01:18 +0200, Michael Tremer <michael.tremer@ipfire.org> wrote:
> >
> >> Hi,
> >>
> >> this patch looks okay, but it won't apply. Git error:
> >>
> >> Applying: Aw: Re: [PATCH] Fix hardcoded strings in pppsetup.cgi
> >> error: patch failed: html/cgi-bin/pppsetup.cgi:793
> >> error: html/cgi-bin/pppsetup.cgi: patch does not apply
> >> error: patch failed: langs/de/cgi-bin/de.pl:1822
> >> error: langs/de/cgi-bin/de.pl: patch does not apply
> >> error: patch failed: langs/en/cgi-bin/en.pl:1852
> >> error: langs/en/cgi-bin/en.pl: patch does not apply
> >> Patch failed at 0001 Aw: Re: [PATCH] Fix hardcoded strings in pppsetup.cgi
> >>
> >> This is probably caused by copy-and-paste and replacing tabs by spaces
> >> or similar.
> >
> 
> (now again without hitting Ctrl+Enter too early... damn)
> 
> I saved the patch from your last reply and compared it to the original patch file
> that was created by TortoiseGit. There were no differences.
> 
> I also patched pppsetup.cgi in my VM with Core 90 (didn´t try for the lang files)
> and this also went fine.
> 
> There are no tabs in the patch.
> 
> I don´t know what could be wrong with it... =(

I think I found it. I do not really know where this is coming from
though.

If I remove the first space from every line of the diff that does not
start with a + or - character. Any idea how this comes?

Also merged.

-Michael
  
Lars Schuhmacher June 5, 2015, 6:52 a.m. UTC | #5
(I changed the subject as this has nothing to do with pppsetup.cgi)


On Thu, 04 Jun 2015 18:23:07 +0200, Michael Tremer <michael.tremer@ipfire.org> wrote:

> If I remove the first space from every line of the diff that does not
> start with a + or - character. Any idea how this comes?


TL;DR: Opera 12.17 f*cks up the patch when sent as plain text.


1) I took the file that TortoiseGit saved, saved the text I sent to the list, the text
I received from the list, and the file that Michael sent back.
2) Edited all of them to start with these lines:
--- a/html/cgi-bin/vpnmain.cgi
+++ b/html/cgi-bin/vpnmain.cgi
3) Converted the saved mails to UNIX format with LF instead of CRLF (Windows here)
4) Calculated md5sums: They were all the same except for the one sent to the list

Examining this further, I noticed a bug in my mail program when it displays the mail
in the "sent" folder: When there is more then one line beginning with a space char,
the mailer will put a superfluous space character into the beginning of the last line.
Strange as hell, but this only affects the display here.

Then, I switched views to have the header displayed, too, and saved it as a file.
Compared to the file created by TortoiseGit and now I can solve this riddle:
There we have superfluous whitespace in front of every line without a + or -
(as you noticed before).

I now also compared the mail I received as it is shown in my mail program and the bug
is present there, too. Display is ok (even without that one extra space char - that is
why the md5sum was ok first), but not when I display it with headers (raw view so to
say).

Will send the next patches with Thunderbird and hope that it will be ok then.
Wasted about an hour examining this - hope you guys at least had fun reading ;-)


Lars
  
Michael Tremer June 5, 2015, 11:25 p.m. UTC | #6
On Thu, 2015-06-04 at 22:52 +0200, Larsen wrote:
> (I changed the subject as this has nothing to do with pppsetup.cgi)
> 
> 
> On Thu, 04 Jun 2015 18:23:07 +0200, Michael Tremer <michael.tremer@ipfire.org> wrote:
> 
> > If I remove the first space from every line of the diff that does not
> > start with a + or - character. Any idea how this comes?
> 
> 
> TL;DR: Opera 12.17 f*cks up the patch when sent as plain text.
> 
> 
> 1) I took the file that TortoiseGit saved, saved the text I sent to the list, the text
> I received from the list, and the file that Michael sent back.
> 2) Edited all of them to start with these lines:
> --- a/html/cgi-bin/vpnmain.cgi
> +++ b/html/cgi-bin/vpnmain.cgi
> 3) Converted the saved mails to UNIX format with LF instead of CRLF (Windows here)

This was one of my first guesses, but it turned out to be false very
quickly.

> 4) Calculated md5sums: They were all the same except for the one sent to the list
> 
> Examining this further, I noticed a bug in my mail program when it displays the mail
> in the "sent" folder: When there is more then one line beginning with a space char,
> the mailer will put a superfluous space character into the beginning of the last line.
> Strange as hell, but this only affects the display here.
> 
> Then, I switched views to have the header displayed, too, and saved it as a file.
> Compared to the file created by TortoiseGit and now I can solve this riddle:
> There we have superfluous whitespace in front of every line without a + or -
> (as you noticed before).

I cannot really understand why this would make the email better readable
or anything.

> I now also compared the mail I received as it is shown in my mail program and the bug
> is present there, too. Display is ok (even without that one extra space char - that is
> why the md5sum was ok first), but not when I display it with headers (raw view so to
> say).
> 
> Will send the next patches with Thunderbird and hope that it will be ok then.
> Wasted about an hour examining this - hope you guys at least had fun reading ;-)
> 
Let us know about your experience with Thunderbird.

-Michael

> 
> Lars
  
Lars Schuhmacher June 5, 2015, 11:29 p.m. UTC | #7
On Fri, 05 Jun 2015 15:25:32 +0200, Michael Tremer  
<michael.tremer@ipfire.org> wrote:

>> Then, I switched views to have the header displayed, too, and saved it  
>> as a file.
>> Compared to the file created by TortoiseGit and now I can solve this  
>> riddle:
>> There we have superfluous whitespace in front of every line without a +  
>> or -
>> (as you noticed before).
>
> I cannot really understand why this would make the email better readable
> or anything.

I know, it´s absolutely stupid. The dev must have had a bad day or  
something...


> Let us know about your experience with Thunderbird.

http://patchwork.ipfire.org/patch/15/ was sent with TB.
Ignoring the aforementioned points that patch is still missing, would it  
theoretically merge?


Lars