Replace/remove redundant methods

[vsajip]

https://github.com/congo-cc/congo-parser-generator/blob/b6d2e1eae55b33e1c8f7a1bb37ed0604c5fd4230/src/templates/java/Lexer.java.ftl#L441-L443 When I do the lexer refactoring, can I assume that this method can be removed and the single use of it (in CSharpLexer.ccc) be changed to use setIgnoredRange instead? I'm assuming I can, but just checking ...

[vsajip]

Also, this seems to not be used: https://github.com/congo-cc/congo-parser-generator/blob/6fa4f254911d7158d3b2c5e0922aea1eaac56637/src/templates/java/TokenSource.java.ftl#L459-L466

[revusky]

When I do the lexer refactoring, can I assume that this method can be removed and the single use of it

Uhh, yeah, I think so. This is a rather strange state of affairs. Of course, there shouldn't be two methods that do the same thing. I think the way this came about is that I wrote the method a second time because I forgot that I already had written the method! And then I guess I later made one an alias of the other because I realized there were two identical methods. As for why I didn't just clean the whole thing up, it may be that it had to be done in two steps because of rebootstrapping, and I only did the first step and forgot to do the second one. Something like that. I can't remember honestly.

Of course, there is the (probabliy slight) possibility that somebody could be using either method name in a code action in some grammar. So, well, arguably, we could leave the setRegionIgnore() method simply because, since it is private, it won't be generated if it's unused. (At least in Java, since we have the Reaper.)

Well, really, the bottom line, I guess, is just clean it up if you want. All of this setIgnoredRange() stuff relates to quite advanced usage actually, like if somebody wanted to implement their own preprocessor or something like that. Unless you've set USE_PREPROCESSOR=true in settings then none of that stuff is generated anyway. I wonder how many end users of this thing even know about this!

[vsajip]

I also noticed that deprecated Node.addChild() is used internally - shouldn't we at least remove all internal uses? it currently warns about deprecations whenever our own generated code is compiled.

[vsajip]

Of course, there is the (probably slight) possibility that somebody could be using either method name in a code action in some grammar

Maybe @adMartem could weigh in if he's using it. Otherwise, I say comment it out, and bring it back if someone complains.

[revusky]

I also noticed that deprecated Node.addChild() is used internally - shouldn't we at least remove all internal uses? it currently warns about deprecations whenever our own generated code is compiled.

Yes, you're right. Feel free to address this. This sort of thing is not controversial at all!

[revusky]

Of course, there is the (probably slight) possibility that somebody could be using either method name in a code action in some grammar

Maybe @adMartem could weigh in if he's using it. Otherwise, I say comment it out, and bring it back if someone complains.

Well, the truth of the matter is that it's rather unlikely. The only way you would be using setRegionIgnore is if you are implementing some sort of preprocessor functionality. Anybody who is that much of a power user (assuming the person exists!) would probably not have too much problem if a superfluous method is removed anyway. So really, don't even worry about it. Just go ahead and fix that if you want.

[revusky]

Also, this seems to not be used:

https://github.com/congo-cc/congo-parser-generator/blob/6fa4f254911d7158d3b2c5e0922aea1eaac56637/src/templates/java/TokenSource.java.ftl#L459-L466

Well, I guess it's true that it's not being used anywhere internally. (I must have been using it at some earlier point.) But my tendency would be to leave that there. I mean, it looks like it is potentially useful in code actions, to know what the length of a line is. Granted, it's a marginal decision, but I would leave it there. And again, there is no absolute guarantee that nobody out there is using it in a code action in their grammar. And, actually, that's quite a bit more likely than with the setIgnoredRegion, which is specificially for doing preprocessor stuff.

So, I would tend to say leave it be...

[adMartem]

For my part, I think I've already eliminated all the addChilds in my stuff. I was excited when Nodes became Lists in part because it makes it much easier to look at nodes in a meaningful way when debugging with a normal view (in Eclipse). Anyway, if there are any residual uses I will change them so I have no problem with any of these changes.

[vsajip]

Closing, as I've made the relevant changes.