Community
Participate
Working Groups
20051115 If I understand right, CompilationUnit.getPosition() returns a character index given line and colunm. The spec says that '0' is returned when e.g. arguments are invalid or no line number available or out of range. But isn't 0 also a valid character index? (for getPosition(1, 0)). How would I tell apart? Why not using -1 as the 'invalid' result as it is more common in Eclipse? Also nasty is the clause 'if no line information is available'. How can I tell? Is this API really useful if I can't rely on it?
*** Bug 116471 has been marked as a duplicate of this bug. ***
What do you suggest for the second issue with the clause 'if no line information is available'? I agree that -1 is a better value when arguments are invalid. We could also throw an IllegalArgumentException. This API is a push down from the apt branch. So I need to make sure this is allright to make changes.
Theodora, Any suggestion?
The "return 0" for invalid argument is just to line up with the behavior of columnNumber(int) and lineNumber(int), which also return 0 if the parameter is invalid or no line number information is available. We could return -1 for invalid argument and -2 if the line number information is not available. If that's the case, I would like to see columNumber(int) and lineNumber(int) behave in a similar manner.
We cannot change lineNumber(int). This would be an API breakage, but it is possible to change the two other APIs. Martin?
I know that lineNumber has been around since the beginning of the AST, but we never used it in JDT UI. Mostly also as the usage of '1' for invalid arguments makes it difficult to use. What about marking lineNumber as deprecated and replace it with a nicer variant? -> getLineNumber(int): -1 if the index is out of bounds, -2 if no line number information is available -> getColumnNumber(int, int): -1 if the index is out of bounds, -2 if no line number information is available ...
Jim, any comment?
Theodora, would the three new APIs be fine for you? I would move them to the apt branch if they are released in HEAD to be consistent.
I will release the changes suggested by Martin in comment 6. If you have anything to say, please speak up.
New API are released in HEAD only for now. Please review them. If ok, then I will close this PR as FIXED and port them to the APT branch.
Consider ok. Fixed and released in HEAD. I will port them to the APT branch. Theodora, could you please then use these new API? I'll deprecated the old API in the APT branch.
Merged into APT_32 branch. Theodora, could you please take care of the changes required after this change? This mostly requires additional bounds check on the value is returned (-1 or -2).
Currently, I don't think we have branched the APT plugin to work with the jdt.core in the APT_32 branch. Is there any plans on pushing it into the APT branch (the one with the 3.1.1 jdt.core)?
Verified for 3.2 M4 using build I20051212-0010
