gitweb showing slash r at the end of line

gitweb showing slash r at the end of line

[Ondra Medek]

Hi,
we have gitweb running on Linux box. Some files have Windows line ending
(CRLF) end we do not use core.autcrlf translation. gitweb show the last \r
in the end of each line, which is annoying. I have creates a simple patch to
avoid this. It adds just one line. I am not sure if the regexp should
contain 'g' switch in the end. Also, not sure if there shoul be some config
option to switch on/off this?

Cheers
Ondra
---
 gitweb/gitweb.perl |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index abb5a79..92175bc 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -1500,6 +1500,7 @@ sub esc_html {
        if ($opts{'-nbsp'}) {
                $str =~ s/ / /g;
        }
+       $str =~ s/\r$//;
        $str =~ s|([[:cntrl:]])|(($1 ne "\t") ? quot_cec($1) : $1)|eg;
        return $str;
 }
--
1.7.8.4


--
View this message in context: http://git.661346.n2.nabble.com/gitweb-showing-slash-r-at-the-end-of-line-tp7229895p7229895.html
Sent from the git mailing list archive at Nabble.com.

Re: gitweb showing slash r at the end of line

[Jakub Narebski]

Ondra Medek <xmedeko@gmail.com> writes:

> we have gitweb running on Linux box. Some files have Windows line ending
> (CRLF) end we do not use core.autcrlf translation. gitweb show the last \r
> in the end of each line, which is annoying.

Well, this "\r" allows to recognize when file with Windows line ending
(CRLF) made it into repository... which usually is discouraged.  But
if you allow this, I can understand that those "\r" at the end of
every line can be annoying.

A simple _workaround_ would be to create one's own extra stylesheet,
with rule hiding control characters visualization (including "\r"), e.g.

  .cntrl { display: none; }

and stuff thys additional it in @stylesheets via gitweb config file.

> I have created a simple patch to avoid this.

Please read Documentation/SubmittingPatches if you want your work to
be considered for inclusion.  This is not a proper patch -- it lacks
commit message (comments should be outside of it, e.g. between "---"
and diffstat) and signoff.

> It adds just one line. I am not sure if the regexp should
> contain 'g' switch in the end. Also, not sure if there should be some config
> option to switch on/off this?

Note that your patch beside hiding "\r" in the case when file has
Windows line endings, it also hides "\r" in the case where file has
_mixed_ line endings (LF mixed with CRLF, which is incorrect).  Though
handling that well would be quite difficult, I think...

Though esc_html gets passed whole lines, I am not sure if it always
gets passed whole lines and would continue getting passed only whole
lines in the future, so there should be 'g' modifier (or better 'gm'
modifier to make sure that $ matches end of line not only end of
string).

I think it would be better if there was an option to switch hiding
"\r" on and off... though I am not sure if it can be done without
incuring large performance penalty.

> ---
>  gitweb/gitweb.perl |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index abb5a79..92175bc 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -1500,6 +1500,7 @@ sub esc_html {
>         if ($opts{'-nbsp'}) {
>                 $str =~ s/ /&nbsp;/g;
>         }
> +       $str =~ s/\r$//;
>         $str =~ s|([[:cntrl:]])|(($1 ne "\t") ? quot_cec($1) : $1)|eg;
>         return $str;
>  }
> --

Another solution would be to modify quot_cec...

-- 
Jakub Narebski

Re: gitweb showing slash r at the end of line

[Junio C Hamano]

Jakub Narebski <jnareb@gmail.com> writes:

> Ondra Medek <xmedeko@gmail.com> writes:
>
>> we have gitweb running on Linux box. Some files have Windows line ending
>> (CRLF) end we do not use core.autcrlf translation. gitweb show the last \r
>> in the end of each line, which is annoying.
>
> Well, this "\r" allows to recognize when file with Windows line ending
> (CRLF) made it into repository... which usually is discouraged.  But
> if you allow this, I can understand that those "\r" at the end of
> every line can be annoying.

I think the right thing to do is:

 * If the repository data is _supposed_ to have CRLF endings (e.g. check
   with core.crlf or something), strip \r and do not show them.

 * Otherwise, i.e. if the repository data is supposed _not_ to have CRLF
   endings, do show these '\r'.  Annoyance here is a *feature* to remind
   the viewer that the contents needs _fixing_.

 * No other switches.

Re: gitweb showing slash r at the end of line

[Jakub Narebski]

On Sat, 28 Jan 2012, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
>> Ondra Medek <xmedeko@gmail.com> writes:
>>
>>> we have gitweb running on Linux box. Some files have Windows line ending
>>> (CRLF) end we do not use core.autcrlf translation. gitweb show the last \r
>>> in the end of each line, which is annoying.
>>
>> Well, this "\r" allows to recognize when file with Windows line ending
>> (CRLF) made it into repository... which usually is discouraged.  But
>> if you allow this, I can understand that those "\r" at the end of
>> every line can be annoying.
> 
> I think the right thing to do is:
> 
>  * If the repository data is _supposed_ to have CRLF endings (e.g. check
>    with core.crlf or something), strip \r and do not show them.
> 
>  * Otherwise, i.e. if the repository data is supposed _not_ to have CRLF
>    endings, do show these '\r'.  Annoyance here is a *feature* to remind
>    the viewer that the contents needs _fixing_.
> 
>  * No other switches.

I agree that it would be a bast solution if gitweb could automatically
infer whether CRLF is allowed (whitelist) or disallowed (blacklist) in
files in given repository.  

But I am not sure if it is possible and what rules there should be for
a *BARE* repository; crlf and eol gitattributes and config variables
are about what should appear in working area - something gitweb is not
interested in at all.

If gitweb code was structured in different way, we could check if all
lines end in LF or all lines end in CRLF and add a note about that to
file or diff header, showing "\r" only in case of mixed line endings...
But that's a futile wish, for now at least.

-- 
Jakub Narebski
Poland

Re: gitweb showing slash r at the end of line

[Ondra Medek]

Hi Jakub,
I have read "SubmittingPatches". I have made a path by "git format-patch -M"
and I have though it's enough. The problem maybe was, that I had not
included "Subject: " from the result of "git format-patch -M". Next time I
will try to do it better.

I am a Git newbie, but my bare repos have "config" file and this file can
contain the "core.autocrlf" setting. So the gitweb can read it. Or what
about to have a special section [gitweb] in this config? For now, the gitweb
config files are somewhat "scattered" = "descrition", "cloneurl",
"project.list", ...

Yeah, the autodetection of mixed mode line endings could be the best
solution.

However, from my point of view a global gitweb setting would be enough for
now.


--
View this message in context: http://git.661346.n2.nabble.com/gitweb-showing-slash-r-at-the-end-of-line-tp7229895p7235866.html
Sent from the git mailing list archive at Nabble.com.

Re: gitweb showing slash r at the end of line

[Jakub Narebski]

Ondra Medek <xmedeko@gmail.com> writes:

> I have read "SubmittingPatches". I have made a path by "git format-patch -M"
> and I have though it's enough. The problem maybe was, that I had not
> included "Subject: " from the result of "git format-patch -M". Next time I
> will try to do it better.

The problem is that in place of proper commit message, describing
change for posteriority as described in SubmittingPatches, you have an
email describing why you created this commit:

OM> Hi,
OM> we have gitweb running on Linux box. Some files have Windows line ending
OM> (CRLF) end we do not use core.autcrlf translation. gitweb show the last \r
OM> in the end of each line, which is annoying. I have creates a simple patch to
OM> avoid this. It adds just one line. I am not sure if the regexp should
OM> contain 'g' switch in the end. Also, not sure if there shoul be some config
OM> option to switch on/off this?
OM> 
OM> Cheers
OM> Ondra
 
Take a look how other commit messages are written in git.git, and
please remember to sign off your patches.

> I am a Git newbie, but my bare repos have "config" file and this file can
> contain the "core.autocrlf" setting. So the gitweb can read it. Or what
> about to have a special section [gitweb] in this config? For now, the gitweb
> config files are somewhat "scattered" = "descrition", "cloneurl",
> "project.list", ...

The [gitweb] section contains stuff that can be used instead of
individual files like 'description' ('gitweb.description' can be used
instead if that file is not present), and also to configure
overridable features on per-repository basis, like 'gitweb.snapshot'.
 
> Yeah, the autodetection of mixed mode line endings could be the best
> solution.
> 
> However, from my point of view a global gitweb setting would be enough for
> now.

I think that if not using autodetection this should be made into
proper gitweb %feature, e.g. named 'eol' - this automatically gives
ability to override it on per-repository basis via repo config.

-- 
Jakub Narebski

Re: gitweb showing slash r at the end of line

[Ondra Medek]

Thanks for pointing out the [gitweb] section of the config. Should I try to
make the patch or someone else more skilled would pick this up?

Cheers
Ondra

--
View this message in context: http://git.661346.n2.nabble.com/gitweb-showing-slash-r-at-the-end-of-line-tp7229895p7237025.html
Sent from the git mailing list archive at Nabble.com.

Re: gitweb showing slash r at the end of line

[Jakub Narebski]

Ondra Medek <xmedeko@gmail.com> writes:

> Thanks for pointing out the [gitweb] section of the config. Should I try to
> make the patch or someone else more skilled would pick this up?

I'll try to pick it up, if you wouldn't be able to...

...subject to finding time for this, of course.

-- 
Jakub Narebski