-
Notifications
You must be signed in to change notification settings - Fork 915
Editor Utilities to LineDocumentUtils migration #8940
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
c872fe9 to
fc386c5
Compare
| /** | ||
| * Get start offset of a (newline character separated) line. | ||
| * @param doc non-null document to operate on | ||
| * @param offset position in document where to start searching | ||
| * @return offset of character right above newline prior the given offset or zero. | ||
| * @throws javax.swing.text.BadLocationException If offset is out of bounds | ||
| */ | ||
| public static int getLineStart2(@NonNull LineDocument doc, int offset) throws BadLocationException { | ||
| checkOffsetValid(doc, offset); | ||
| return doc.getParagraphElement(offset).getStartOffset(); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new method. the old is now deprecated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's ugly. Surely we can get a slightly better name here? Maybe getLineStartOffset / getLineEndOffset or findLineStart / findLineEnd, or even lineStart / lineEnd? Be good to document the line end method while you're at it. Change should probably go in apichanges.xml.
@ebarboni should consider changes required to get this module in Javadoc too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was actually about to rename it, but this got delayed since I noticed while rebasing that the file history tab sorts git commits by AuthorDate not CommitDate, which confused me since I thought i merged into the wrong direction. Something we probably should fix. (git log --pretty=fuller shows both dates)
i like lineStart / lineEnd will update it soon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
on second thought: getLineStartOffset is probably better since it doesn't break the get* naming pattern + the methods return getStartOffset() of javax.swing.text.Element
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it's ugly. Surely we can get a slightly better name here? Maybe
getLineStartOffset/getLineEndOffsetorfindLineStart/findLineEnd, or evenlineStart/lineEnd? Be good to document the line end method while you're at it. Change should probably go inapichanges.xml.@ebarboni should consider changes required to get this module in Javadoc too?
Maybe looks like there is around 20 modules not in apidoc that hava apichanges.xml. Will try to list them. No findings on bz of api/spi move or so but AFAIK editor.document is not in apidoc since the begining.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ebarboni IIRC from the dashboard module it's due to lack of API section in arch - eg. https://github.com/apache/netbeans/blob/master/ide/editor.document/arch.xml#L50 vs https://github.com/apache/netbeans/blob/master/platform/api.dashboard/arch.xml#L55 ??
…Offset avoid using deprecated NB API to reduce warning noise adds getLine[Start/End]Offset and deprecates getLine[Start/End] - getLineStart didn't declare BadLocationException to be thrown and did not make any range checks - getRowStart declared BadLocationException but called getLineStart which can't throw the Exception - getLineStartOffset checks the range and declares BadLocationException just like the getLineEndOffset sibling. - this makes the API symmetrical and allows the catch blocks (which had to be already implemented) to function
210afcf to
b4204ec
Compare
…irst/Last]NonWhitespace and deprecate Utilities::getRowFirstNonWhite for symmetry
|
duplicated code strikes again |
b4204ec to
034d38d
Compare
Switched to non-deprecated editor utilities API.
PR is split into commits which usually handle a method pair.
note to first commit:
LineDocumentUtilsunfortunately broke symmetry ingetLine[Start/End]:getLineStartdidn't declareBadLocationExceptionto be thrown and did not have any range checksgetRowStartdeclaredBadLocationExceptionbut calledgetLineStartwhich can't throw theExceptionto resolve this I had to add
getLine[Start/End]Offsetand deprecategetLine[Start/End]:getLine[Start/End]Offsetchecks the range and declaresBadLocationException, just like thegetLineEndsibling did.its ugly but this was the best option I came up with.