Re: Small fix of column number in parser

[Daniel Kraft]

Hi Jordi,

On 04/28/11 07:54, Jordi Gutiérrez Hermoso wrote:
> On 5 April 2011 04:17, Daniel Kraft<address@hidden>  wrote:
>
> > attached is a hg changeset that fixes wrong column numbers.  If the
> > attached test.m is executed without the patch, one gets:
> >
> > error: bar
> > error: called from:
> > error:   /tmp/test.m at line 2, column -1
> >
> > With the patch, the column number is correctly reported as 1.
>
> I tested your patch and it worked, so I pushed it to the default
> branch:
>
>      http://hg.savannah.gnu.org/hgweb/octave/rev/fd367312095a

thanks!  Regarding testing a patch before commit (or submitting it), is 
there some "standard required way" -- i.e., running "make check" without 
errors or something else?

> > Do you prefer this simple patch or should I work on some "clean-up"
> > of this situation, too -- like making skip_white_space a member of
> > stdio_stream_reader and unifying column- and line-number tracking in
> > one place.
>
> I think for a first patch, a minimal solution is fine. Leave the
> refactoring for later.

Ok.  So should I now work on a refactoring?  While I believe that the 
code in question could become simpler, I also think that it is actually 
only run in very rare circumstances (namely for the white-space at the 
very beginning of a file); so it probably won't be that helpful, after all.

> > PS: Since this is my first attempt at both working with Mercurial
> > and submitting a patch for octave, please let me know if there's
> > something I should do different (and in general, comments welcome).
>
> You used mq; please submit finished patches (commits) next time, and
> use a meaningful commit message. You also used our old-style ChangeLog
> commit format, which we recently changed. Put the same information
> that you put in the ChangeLog in the commit message. Here are the
> guidelines for doing so:
>
>      
> http://octave.1599824.n4.nabble.com/policy-for-pushing-changesets-tp3443512p3445355.html

Ok about the commit message and ChangeLog change.  How can I convert my 
mq queue to a finished patch?  I followed the workflow at 
http://www.gnu.org/software/octave/doc/interpreter/How-to-Contribute.html#How-to-Contribute, 
AFAIK.  (And never used hg before.)

Is it enough to just do a hg commit on the active queue before hg 
export?  And if I do this, can I later get back to the patch, revise it 
and submit a changed version in case something comes up in code review?

> >   In particular, is there some tag I should add to messages sent with
> > patches?  E.g., on the GCC lists, we have "[patch]" in the subjects
> > of messages containing diff's.
>
> Yes, please use our patch tracker. Easier to have them all in one
> place instead of scattered in our respected inboxen:
>
>      https://savannah.gnu.org/patch/?group=octave

Ah, I didn't know about that -- so far I'm used to submitting patches 
for review on the mailing lists.  But in the future I'll happily use the 
patch tracker, seems like a useful thing :)

Thanks a lot,
Daniel

--
http://www.pro-vegan.info/
--
Done:  Arc-Bar-Cav-Kni-Ran-Rog-Sam-Tou-Val-Wiz
To go: Hea-Mon-Pri