Re: [Ctags-devel] Fwd: Various patches (though mostly JavaScript ones)

[Colomban W. <lis...@he...>]

Hi David,

Le 04/12/2012 03:40, David Fishburn a écrit :
> 
> Columban, sorry it took so long to get back to these changes.

No problem, I know free time is quite rare :)

>> I checked the changes you applied against my patches, and here's the
> differences:
> 
>>  * Test/3470609.js (introduces by patch
> 0011-JavaScript-Fix-parsing-non-method-properties.patch and updated by
> patch 0012-JavaScript-Create-class-tag-for-variable-with-childr.patch)
> is missing.
> 
> Fixed.
> 
> Your change added this:
>   * Classes:
> + *    root
> 
> 
> Though, that tag is not generated.

Apparently it's because you (or some tool?) added an UTF-8 BOM to the
test file, and it seems the parser don't handle it correctly.  Removing
the BOM fixed the issue.

However, you seem to added the test case twice, both as Test/3470609.js
(with the BOM) and Test/bug3470609.js (without).

>>  * on jscript.c, line 451, you didn't remove "token->type =
> TOKEN_FORWARD_SLASH".  This is harmless, but the switch right above will
> override the value anyway.
> 
> I cannot find TOKEN_FORWARD_SLASH in any of the 12 patches sent to me.

It's not introduced by my patches, it was already there in jscript.c
(line 101).

But it's the "[JavaScript] Properly parse regular expression literals"
patch that did remove the line I was talking about.

But again, it's not a big deal, it's just one useless assignation the
compiler will probably strip out anyway.

>>  * you didn't apply the changes I made on read.h for the const stuff.
> 
> That is not my parser and I am uncomfortable making changes to it as my
> C knowledge is quite low.  Do any of your changes rely on it?  

OK.  And well, it's not strictly required, but it will produce wrong
line information sometimes.  One of my patches did reduce the error, but
it didn't eliminate it altogether.  To workaround it without touching
read.h requires to call `getSourceLineNumber()` and
`getInputFilePosition()` at *most* once per function -- which would need
a huge refactoring of `readToken()` in jscript.c.

So fixing it will give more accurate line numbers, but that's "all".

> What you can do is suggest that change to ctags-devel and see if someone
> else agrees.  Then change of course must be cross platform.

OK, will do.

>>  * you added one space at the start of many new lines, is this wanted?
> 
> Nope, fixed.
> 
> 
> Sending        Test\1880687.js
> Adding         Test\3470609.js
> Sending        Test\jsFunc_tutorial.js
> Sending        jscript.c
> Transmitting file data ....
> Committed revision 800.
> 
> Dave

Thanks a lot for taking care of these patches!

Regards,
Colomban