Re: cleaning code (was Re: [Patch] Javadoc & style

[Xavier Hanin <xa...@gmail.com>]

On 5/31/07, Matt Benson <gu...@yahoo.com> wrote:
>
> Guys, can I interject a dumb question and ask why
> Xooki must use LF endings?


The question is not dumb, xooki is :-)
The problem is that when you use xooki to edit a page, xooki writes the
page. And when it writes the page, it uses '\n' for eol whatever the
platform is. Thus you end up with LF whatever your platform, so using
eol-style 'native' does not work with xooki.

But fixing this in xooki shouldn't be too long, so if it's a problem to use
LF as eol-style for these pages, I'll try to fix xooki.

Xavier

-Matt
>
>
>
>
>
> ____________________________________________________________________________________
> Get the Yahoo! toolbar and be alerted to new email wherever you're
> surfing.
> http://new.toolbar.yahoo.com/toolbar/features/mail/index.php
>



-- 
Xavier Hanin - Independent Java Consultant
Manage your dependencies with Ivy!
http://incubator.apache.org/ivy/

Re: cleaning code (was Re: [Patch] Javadoc & sty

[Xavier Hanin <xa...@gmail.com>]

On 5/31/07, Matt Benson <gu...@yahoo.com> wrote:
>
>
> [SNIP]
> > I think I'll fix xooki to make use of native eol, it
> > will make our lifes
> > easier. Does it sound right?
> >
>
> From your other comments, I'd agree this would
> probably be the easiest solution in the long run.  :)


I've made a fix checked in xooki only for the moment. I'll upgrade to this
fixed version and convert all files to native eol when I'll clean up the
code next week, if nobody's object.

Xavier

-Matt
>
> > Xavier
> >
> > -Matt
> > >
> > > >
> > > > Xavier
> > > >
> > > > -Matt
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > >
> > >
> >
>
> ____________________________________________________________________________________
> > > > > Get the Yahoo! toolbar and be alerted to new
> > email
> > > > wherever you're
> > > > > surfing.
> > > > >
> > > >
> > >
> >
> http://new.toolbar.yahoo.com/toolbar/features/mail/index.php
> > > > >
> > > >
> > > >
> > > >
> > > > --
> > > > Xavier Hanin - Independent Java Consultant
> > > > Manage your dependencies with Ivy!
> > > > http://incubator.apache.org/ivy/
> > > >
> > >
> > >
> > >
> > >
> > >
> > >
> >
>
> ____________________________________________________________________________________
> > > Sick sense of humor? Visit Yahoo! TV's
> > > Comedy with an Edge to see what's on, when.
> > > http://tv.yahoo.com/collections/222
> > >
> >
> >
> >
> > --
> > Xavier Hanin - Independent Java Consultant
> > Manage your dependencies with Ivy!
> > http://incubator.apache.org/ivy/
> >
>
>
>
>
>
> ____________________________________________________________________________________
> Get the Yahoo! toolbar and be alerted to new email wherever you're
> surfing.
> http://new.toolbar.yahoo.com/toolbar/features/mail/index.php
>



-- 
Xavier Hanin - Independent Java Consultant
Manage your dependencies with Ivy!
http://incubator.apache.org/ivy/

Re: cleaning code (was Re: [Patch] Javadoc & styl

[Gilles Scokart <gs...@gmail.com>]

2007/5/31, Xavier Hanin <xa...@gmail.com>:
>
> I think I'll fix xooki to make use of native eol, it will make our lifes
> easier. Does it sound right?
>
> Xavier

Yes, it's better if xooki use native EOL.  It will allow to set the
svn eol-style property to native, which is the default value for other
apache project.

Note that the problem is also visible when providing diff when you are
on a windows platform.  Indeed, with a svn-style property set to
native, the original files use CRLF.  After a modification (before the
xooki fix) the file contains only LF.  A diff without the correct
parameter will say that all the text has changed.

The two solution to that are :
- put LF in the svn property with xooki using LF (the current situation)
- put native in the svn property with xooki using native EOL.

The second is better as it follows the apache standard for html files.

-- 
Gilles SCOKART

Re: cleaning code (was Re: [Patch] Javadoc & styl

[Matt Benson <gu...@yahoo.com>]

--- Xavier Hanin <xa...@gmail.com> wrote:

> On 5/31/07, Matt Benson <gu...@yahoo.com>
> wrote:
> >
> >
> > --- Xavier Hanin <xa...@gmail.com> wrote:
> >
> > > On 5/31/07, Matt Benson <gu...@yahoo.com>
> > > wrote:
> > > >
> > > > Guys, can I interject a dumb question and ask
> why
> > > > Xooki must use LF endings?
> > >
> > >
> > > The question is not dumb, xooki is :-)
> > > The problem is that when you use xooki to edit a
> > > page, xooki writes the
> > > page. And when it writes the page, it uses '\n'
> for
> > > eol whatever the
> > > platform is. Thus you end up with LF whatever
> your
> > > platform, so using
> > > eol-style 'native' does not work with xooki.
> > >
> > > But fixing this in xooki shouldn't be too long,
> so
> > > if it's a problem to use
> > > LF as eol-style for these pages, I'll try to fix
> > > xooki.
> >
> > My understanding of the svn property is that it
> will
> > store the files however it sees fit and on
> checkout
> > will give them to the user in his native EOL. 
> IOW,
> > there is no conflict.
> 
> 
>  I'm not sure to properly understand the
> svn:eol-style property, but from my
> experience I have at least one problem when using
> native eol: xooki rewrites
> only a part of the original file with LF, so the
> file after xooki edition
> has a mix of CRLF and LF if the original file had
> CRLF. And svn do not seem
> to like mixed eol styles when the eol-style property
> is set.
> 

I wasn't aware of this, so took a more simplistic view
of the situation.  :)

[SNIP]
> I think I'll fix xooki to make use of native eol, it
> will make our lifes
> easier. Does it sound right?
> 

From your other comments, I'd agree this would
probably be the easiest solution in the long run.  :)

-Matt

> Xavier
> 
> -Matt
> >
> > >
> > > Xavier
> > >
> > > -Matt
> > > >
> > > >
> > > >
> > > >
> > > >
> > > >
> > >
> >
> >
>
____________________________________________________________________________________
> > > > Get the Yahoo! toolbar and be alerted to new
> email
> > > wherever you're
> > > > surfing.
> > > >
> > >
> >
>
http://new.toolbar.yahoo.com/toolbar/features/mail/index.php
> > > >
> > >
> > >
> > >
> > > --
> > > Xavier Hanin - Independent Java Consultant
> > > Manage your dependencies with Ivy!
> > > http://incubator.apache.org/ivy/
> > >
> >
> >
> >
> >
> >
> >
>
____________________________________________________________________________________
> > Sick sense of humor? Visit Yahoo! TV's
> > Comedy with an Edge to see what's on, when.
> > http://tv.yahoo.com/collections/222
> >
> 
> 
> 
> -- 
> Xavier Hanin - Independent Java Consultant
> Manage your dependencies with Ivy!
> http://incubator.apache.org/ivy/
> 



       
____________________________________________________________________________________
Get the Yahoo! toolbar and be alerted to new email wherever you're surfing.
http://new.toolbar.yahoo.com/toolbar/features/mail/index.php

Re: cleaning code (was Re: [Patch] Javadoc & styl

[Xavier Hanin <xa...@gmail.com>]

On 5/31/07, Matt Benson <gu...@yahoo.com> wrote:
>
>
> --- Xavier Hanin <xa...@gmail.com> wrote:
>
> > On 5/31/07, Matt Benson <gu...@yahoo.com>
> > wrote:
> > >
> > > Guys, can I interject a dumb question and ask why
> > > Xooki must use LF endings?
> >
> >
> > The question is not dumb, xooki is :-)
> > The problem is that when you use xooki to edit a
> > page, xooki writes the
> > page. And when it writes the page, it uses '\n' for
> > eol whatever the
> > platform is. Thus you end up with LF whatever your
> > platform, so using
> > eol-style 'native' does not work with xooki.
> >
> > But fixing this in xooki shouldn't be too long, so
> > if it's a problem to use
> > LF as eol-style for these pages, I'll try to fix
> > xooki.
>
> My understanding of the svn property is that it will
> store the files however it sees fit and on checkout
> will give them to the user in his native EOL.  IOW,
> there is no conflict.


 I'm not sure to properly understand the svn:eol-style property, but from my
experience I have at least one problem when using native eol: xooki rewrites
only a part of the original file with LF, so the file after xooki edition
has a mix of CRLF and LF if the original file had CRLF. And svn do not seem
to like mixed eol styles when the eol-style property is set.

  Xooki can do whatever it wants
> to your templates, you can check them in (with
> eol-style set) and everybody will be happy.  This
> presumes that Xooki can handle as input any eol thrown
> at it.


Actually after some investigation it seems that the problem come from the
string returned by the browser when you get a textarea input value. Xooki
get the value of the text area like this:
this._source = document.getElementById('xooki-source').value
And this return a string with LF eol whatever the original file contains. So
I think it can handle any eol the browser handle, but it will always see
them as LF (at least on firefox).
So fixing the problem in xooki is not that easy. We could choose to output
platform native eol whatever the original eol was. In this case it would
make xooki play fine with a svn:eol-style = native property.
This is very easy to do at the time xooki writes to the disk, by replacing
eols by native eol, native eol being selected depending on the
navigator.plaform (CRLF for windows, LF for the rest, and forget about old
macs).

I noticed the \r?\n regex in use in Xooki.js
> so the only problem situation I can foresee ATM is if
> Xooki were used on a platform with \r as its default
> separator as the regex doesn't handle this,


According to my latest tests, it seems that I was a bit too zealous when
writing this regexp, because LF only are present in the input (at least on
firefox 2 and IE 7)

but (1)
> that should be easily fixable, and (2) there are
> probably very few people developing Ivy (or any other
> sf package using Xooki for its documentation ;) ) on
> such a platform.


Indeed :)

WDYT?


I think I'll fix xooki to make use of native eol, it will make our lifes
easier. Does it sound right?

Xavier

-Matt
>
> >
> > Xavier
> >
> > -Matt
> > >
> > >
> > >
> > >
> > >
> > >
> >
>
> ____________________________________________________________________________________
> > > Get the Yahoo! toolbar and be alerted to new email
> > wherever you're
> > > surfing.
> > >
> >
> http://new.toolbar.yahoo.com/toolbar/features/mail/index.php
> > >
> >
> >
> >
> > --
> > Xavier Hanin - Independent Java Consultant
> > Manage your dependencies with Ivy!
> > http://incubator.apache.org/ivy/
> >
>
>
>
>
>
> ____________________________________________________________________________________
> Sick sense of humor? Visit Yahoo! TV's
> Comedy with an Edge to see what's on, when.
> http://tv.yahoo.com/collections/222
>



-- 
Xavier Hanin - Independent Java Consultant
Manage your dependencies with Ivy!
http://incubator.apache.org/ivy/

Re: cleaning code (was Re: [Patch] Javadoc & style

[Matt Benson <gu...@yahoo.com>]

--- Xavier Hanin <xa...@gmail.com> wrote:

> On 5/31/07, Matt Benson <gu...@yahoo.com>
> wrote:
> >
> > Guys, can I interject a dumb question and ask why
> > Xooki must use LF endings?
> 
> 
> The question is not dumb, xooki is :-)
> The problem is that when you use xooki to edit a
> page, xooki writes the
> page. And when it writes the page, it uses '\n' for
> eol whatever the
> platform is. Thus you end up with LF whatever your
> platform, so using
> eol-style 'native' does not work with xooki.
> 
> But fixing this in xooki shouldn't be too long, so
> if it's a problem to use
> LF as eol-style for these pages, I'll try to fix
> xooki.

My understanding of the svn property is that it will
store the files however it sees fit and on checkout
will give them to the user in his native EOL.  IOW,
there is no conflict.  Xooki can do whatever it wants
to your templates, you can check them in (with
eol-style set) and everybody will be happy.  This
presumes that Xooki can handle as input any eol thrown
at it.  I noticed the \r?\n regex in use in Xooki.js
so the only problem situation I can foresee ATM is if
Xooki were used on a platform with \r as its default
separator as the regex doesn't handle this, but (1)
that should be easily fixable, and (2) there are
probably very few people developing Ivy (or any other
sf package using Xooki for its documentation ;) ) on
such a platform.

WDYT?

-Matt

> 
> Xavier
> 
> -Matt
> >
> >
> >
> >
> >
> >
>
____________________________________________________________________________________
> > Get the Yahoo! toolbar and be alerted to new email
> wherever you're
> > surfing.
> >
>
http://new.toolbar.yahoo.com/toolbar/features/mail/index.php
> >
> 
> 
> 
> -- 
> Xavier Hanin - Independent Java Consultant
> Manage your dependencies with Ivy!
> http://incubator.apache.org/ivy/
> 



       
____________________________________________________________________________________
Sick sense of humor? Visit Yahoo! TV's 
Comedy with an Edge to see what's on, when. 
http://tv.yahoo.com/collections/222