You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Erik Huelsmann <eh...@gmail.com> on 2006/06/11 08:00:48 UTC

[RFC] 1.4.x platform dependent blame output

Hi!

As part of solving issue #2431 (blame ignores svn:eol-style), blame
now gives different output  for (some eol) -> native svn:eol-style
changes. Brane, Lundblad and I all agree this is not correct.

There is also another issue: I actually think blame output is client
output (like svn status) rather than raw output (like svn cat). This
means the issue is invalid and needs no fixing: we just need to make
sure we generate the right platform specific eols.

The data passed to the blame callback is insufficiently defined to
guarantee the right platform specific eol style, because it doesn't
say whether the file-eols are in- or excluded from the line data. In
practice, we pass CRs when the eol style is CRLF.

If the eol style is CR we give completely broken results: the full
file is passed as line 1.

I'd like your comments to this:

- blame output is 'client output', not 'raw output'
- blame output should be the same for all platforms
- we should fix the callback to use the right eol style
- we should fix the line data to be better defined
- all the above needs to be done before 1.4.0 final (I have the patches)

Thanks for your comments!

bye,

Erik.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [RFC] 1.4.x platform dependent blame output

Posted by Erik Huelsmann <eh...@gmail.com>.
On 6/11/06, D.J. Heap <dj...@gmail.com> wrote:
> On 6/11/06, Erik Huelsmann <eh...@gmail.com> wrote:
> > Hi!
> >
> > As part of solving issue #2431 (blame ignores svn:eol-style), blame
> > now gives different output  for (some eol) -> native svn:eol-style
> > changes. Brane, Lundblad and I all agree this is not correct.
> >
> > There is also another issue: I actually think blame output is client
> > output (like svn status) rather than raw output (like svn cat). This
> > means the issue is invalid and needs no fixing: we just need to make
> > sure we generate the right platform specific eols.
> >
> > The data passed to the blame callback is insufficiently defined to
> > guarantee the right platform specific eol style, because it doesn't
> > say whether the file-eols are in- or excluded from the line data. In
> > practice, we pass CRs when the eol style is CRLF.
> >
> > If the eol style is CR we give completely broken results: the full
> > file is passed as line 1.
> >
> > I'd like your comments to this:
> >
> > - blame output is 'client output', not 'raw output'
>
>
> That seems like the natural thing to do to me -- I most often see
> blame output sent to a file and loaded into a text editor to be looked
> at more closely.

I have a patch which does this ...

> > - blame output should be the same for all platforms

... and this ...

> Does this mean in contrast to the above or something else?

What I mean by that item is: if a files svn:eol-style is switched from
LF -> native, that should show the same blame output on CRLF platforms
as on LF-only platforms. Currently, they give different outputs.

> > - we should fix the callback to use the right eol style
> > - we should fix the line data to be better defined
> > - all the above needs to be done before 1.4.0 final (I have the patches)
>
>
> Fixing the callback and the line data in the callback to be well
> defined seems like a very reasonable thing for a 3rd party client of
> the libraries to expect, so I would say yes.

... and this ...

So, I'll go and post the patch if no other reactions come in tomorrow.

Thanks for your comments!

bye,

Erik.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [RFC] 1.4.x platform dependent blame output

Posted by "D.J. Heap" <dj...@gmail.com>.
On 6/11/06, Erik Huelsmann <eh...@gmail.com> wrote:
> Hi!
>
> As part of solving issue #2431 (blame ignores svn:eol-style), blame
> now gives different output  for (some eol) -> native svn:eol-style
> changes. Brane, Lundblad and I all agree this is not correct.
>
> There is also another issue: I actually think blame output is client
> output (like svn status) rather than raw output (like svn cat). This
> means the issue is invalid and needs no fixing: we just need to make
> sure we generate the right platform specific eols.
>
> The data passed to the blame callback is insufficiently defined to
> guarantee the right platform specific eol style, because it doesn't
> say whether the file-eols are in- or excluded from the line data. In
> practice, we pass CRs when the eol style is CRLF.
>
> If the eol style is CR we give completely broken results: the full
> file is passed as line 1.
>
> I'd like your comments to this:
>
> - blame output is 'client output', not 'raw output'


That seems like the natural thing to do to me -- I most often see
blame output sent to a file and loaded into a text editor to be looked
at more closely.


> - blame output should be the same for all platforms


Does this mean in contrast to the above or something else?


> - we should fix the callback to use the right eol style
> - we should fix the line data to be better defined
> - all the above needs to be done before 1.4.0 final (I have the patches)


Fixing the callback and the line data in the callback to be well
defined seems like a very reasonable thing for a 3rd party client of
the libraries to expect, so I would say yes.

DJ

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

RE: [RFC] 1.4.x platform dependent blame output

Posted by Lieven Govaerts <lg...@mobsol.be>.
Erik,

attached patch is a new blame test that will validate the result of blame
when the eol-style property is set to CR.

It's failing currently, all blame output is returned in one line instead of
three (just like you expected).

Lieven.

> -----Original Message-----
> From: Erik Huelsmann [mailto:ehuels@gmail.com] 
> Sent: zondag 11 juni 2006 10:01
> To: dev
> Subject: [RFC] 1.4.x platform dependent blame output
> 
> Hi!
> 
> As part of solving issue #2431 (blame ignores svn:eol-style), 
> blame now gives different output  for (some eol) -> native 
> svn:eol-style changes. Brane, Lundblad and I all agree this 
> is not correct.
> 
> There is also another issue: I actually think blame output is 
> client output (like svn status) rather than raw output (like 
> svn cat). This means the issue is invalid and needs no 
> fixing: we just need to make sure we generate the right 
> platform specific eols.
> 
> The data passed to the blame callback is insufficiently 
> defined to guarantee the right platform specific eol style, 
> because it doesn't say whether the file-eols are in- or 
> excluded from the line data. In practice, we pass CRs when 
> the eol style is CRLF.
> 
> If the eol style is CR we give completely broken results: the 
> full file is passed as line 1.
> 
> I'd like your comments to this:
> 
> - blame output is 'client output', not 'raw output'
> - blame output should be the same for all platforms
> - we should fix the callback to use the right eol style
> - we should fix the line data to be better defined
> - all the above needs to be done before 1.4.0 final (I have 
> the patches)
> 
> Thanks for your comments!
> 
> bye,
> 
> Erik.
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
> For additional commands, e-mail: dev-help@subversion.tigris.org
>