Re: [PATCH EGIT Allow for git config to not error when lines have '/r' in them.]

Re: [PATCH EGIT Allow for git config to not error when lines have '/r' in them.]

[Robin Rosenberg]

torsdag 08 januari 2009 04:10:05 skrev Ryan Alberts:
> I have attached a small fix for when a git config has /r lines in the file.
> I have to admit that I do not usually submit patches to the open source
> community and I am not very familiar with the process :-)  Please, please,
> let me know if I can do something different next time!

1) Inline the patch
No, need for such a large sign-off. It's good you declare that you have
understand the SOB-line.

You can add "extra" comments that should not go into the cBommit after the
comment like this:

From:.. etc

"This" goes into the commit

Signed-off-by: Whom Ever  <whom.ever@example.com>
--- 

The three dashes ends the comment. Anything after it and the actual patch
will be ignored by git am, but might be useful to the maintainer.

Back to the patch. I think we should only ignore \r (not /r, but could say CR in
a comment) before an LF. 

-- robin

Re: [PATCH EGIT Allow for git config to not error when lines have '/r' in them.]

[Shawn O. Pearce]

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> torsdag 08 januari 2009 04:10:05 skrev Ryan Alberts:
> > I have attached a small fix for when a git config has /r lines in the file.
> > I have to admit that I do not usually submit patches to the open source
> > community and I am not very familiar with the process :-)  Please, please,
> > let me know if I can do something different next time!
> 
> Back to the patch. I think we should only ignore \r (not /r, but could say CR in
> a comment) before an LF. 

I disagree; ignoring CR anyplace in the file should be fine, unless
it is within a double quoted value, in which case it should use
the same rule that \n would use within the same region.

' ', \n, \t all act the same with regards to being (mostly) ignored
whitespace.  \r is no different.  Especially for anyone crazy enough
to still be using a Mac style formatted file, with only CRs in it.

-- 
Shawn.

Re: [PATCH EGIT Allow for git config to not error when lines have '/r' in them.]

[Robin Rosenberg]

"Shawn O. Pearce" <spearce@spearce.org> wrote
> Robin Rosenberg <robin.rosenberg@dewire.com> wrote:
> > torsdag 08 januari 2009 04:10:05 skrev Ryan Alberts:
> > > I have attached a small fix for when a git config has /r lines in the file.
> > > I have to admit that I do not usually submit patches to the open source
> > > community and I am not very familiar with the process :-)  Please, please,
> > > let me know if I can do something different next time!
> > 
> > Back to the patch. I think we should only ignore \r (not /r, but could say CR in
> > a comment) before an LF. 
> 
> I disagree; ignoring CR anyplace in the file should be fine, unless
> it is within a double quoted value, in which case it should use
> the same rule that \n would use within the same region.
> 
> ' ', \n, \t all act the same with regards to being (mostly) ignored
> whitespace.  \r is no different.  Especially for anyone crazy enough
> to still be using a Mac style formatted file, with only CRs in it.

Those are copied from in to out in the repo config parser today. Why
should ' \r' be treated differently? Just CR (FF, Ctrl-G etc) is garbage, 
but CRLF is a well established line terminator.

-- robin