Fixes several bugs in the editor annotation options.#8231
Fixes several bugs in the editor annotation options.#8231mbien merged 1 commit intoapache:masterfrom
Conversation
5adda61 to
d95f30b
Compare
|
there is still something weird going on when annotation colors of copied profiles are persisted. Have to take another look. edit: I think I have found the (hopefully) last remaining problem. Fix in progress. |
d95f30b to
86fe0a9
Compare
|
unfortunately this required more changes than anticipated. But I think the editor annotation color profile options work now as intended I hope. The missing bit is that the inherited colors make only sense if the base theme is well defined. Currently its the "NetBeans" theme which doesn't make a lot of sense for dark themes etc. But fixing this would go beyond the annotations tab. I fear the "Highlighting" tab is also broken, but I need a break from the color settings :) |
| /** | ||
| * Returns annotations properties for given profile or null, if the | ||
| * Returns annotations properties for given profile or an empty Map, if the | ||
| * profile is not known. | ||
| * | ||
| * @param profile a profile name | ||
| * @return annotations properties for given profile or null | ||
| * @return annotations properties for given profile or an empty map | ||
| */ | ||
| public abstract Map<String, AttributeSet> getAnnotations ( |
There was a problem hiding this comment.
the impl never returned null in the last ~20 years. Since it is friend API I decided to change the doc instead of the impl since it seemed to be lower risk.
| kbsChanged = true; | ||
|
|
||
| } else if (evt.getPropertyName() == null) { | ||
| } else if (prop == null) { |
There was a problem hiding this comment.
Just reading the diff, though about using switch over String in this case?
There was a problem hiding this comment.
nothing against it, but it would reformat a big section which would make reviews more difficult and might risk to hide the change in it. The main reason I extracted the prop name is because it would make the added || longer a few lines later.
There was a problem hiding this comment.
Yeah, nice to have minimal changes for reviewing.
|
Some themes have more settings than others in the Highlighting tab, when I play with it. Is that correct? |
|
@eirikbakke this PR affects only the annotations tab. The highlighting tab has likely similar issues and has to be addressed separately at some point in future. |
eirikbakke
left a comment
There was a problem hiding this comment.
I played with the settings, Duplicate, Restore etc;; it seems to work. It's hard to verify correctness by just reading the changes, but I think it's fine to merge.
|
thanks for review. going to rebase the PR for a refresh, then do a last test and merge. |
- Make sure the current editor annotation profile is correctly copied on profile duplication. - Make sure annotation profile settings are correctly persisted and applied on change - All profiles must have annotation color configs to have inherited values work correctly (added missing) - EditorSettings: fix javadoc since the impl never returned null in the last 20 years.
86fe0a9 to
9809a2e
Compare
Fixes several issues with the editor annotation options.
manual test:
options -> fonts & colors -> annotationsfixes #8143 and https://issues.apache.org/jira/browse/NETBEANS-1493