threads / patch / 17082

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

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

## tl;dr

3 messages between Jan 10, 2009 and Jan 12, 2009. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Robin Rosenberg· Jan 10, 2009, 20:34 UTC · lore

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

Shawn O. Pearce· Jan 12, 2009, 17:10 UTC · re: Robin Rosenberg · lore

Robin Rosenberg <robin.rosenberg@dewire.com> wrote:

Show 8 quoted lines

> 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.

Robin Rosenberg· Jan 12, 2009, 22:53 UTC · re: Shawn O. Pearce · lore

"Shawn O. Pearce" <spearce@spearce.org> wrote

Show 17 quoted lines

> 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