More work on centralizing icon loading to ImageUtilities

[eirikbakke]

This is a follow-up on the separate work in #8114 and #8109 . To render at full HiDPI resolution, with SVG icon substitutions where available, Icon/Image instances must be created via the methods in ImageUtilities rather than, in particular, the constructors of ImageIcon.

This PR, combined with the previously mentioned PRs, handles most of the remaining cases.

Specifically:

• Search for new ImageIcon( and rewrite each relevant case to use ImageUtilities to load icons instead.
  • In many cases this means replacing new ImageIcon(SomeClassInCurrentModule.class.getResource("/package/folders/path/to/icon.png")) to ImageUtilities.loadIcon("package/folders/path/to/icon.png"). I verified visually in the IDE that this worked in one case (the Print dialog), to make sure this is a valid transformation. In cases where a relative path was previously used, I replaced it with an absolute path that I looked up manually (this page is helpful for this). Note that ImageUtilities does not want leading slashes.
  • The CompletionProvider API requires ImageIcon to be used. The ImageUtilities.icon2ImageIcon method is used to produce a special version of ImageIcon that supports HiDPI scaling in these cases.
• Search for instanceof ImageIcon and generalize to instanceof Icon when appropriate.
• Search for getLookAndFeel*getDisabledIcon and switch to ImageUtilities.createDisabledIcon.
• Search for loadImageIcon("/ and remove the leading slash in some cases (which would produce a warning log entry). Also switch to loadIcon if possible if I'm already changing these lines.
• Deleted some commented or unused code in some cases, to prevent irrelevant code from showing up when searching for places to refactor.

Places where uses of new ImageIcon were left unmodified include:

• Test classes
• Classes relating to loading and display of arbitrary icons, e.g. the property editor for Icon instances.
• Places where the ImageIcon is simply used as an empty icon.
• Installer-related modules.

It'll be good to test these changes in development builds for a while, in case icon loading breaks accidentally somewhere. I'll start doing so in my own working IDE now. (I already tested the separate changes in #8109 for three weeks.)

[mbien]

looks good. Wondering if you considered using jackpot for (some of) the migration rules?

e.g.: https://github.com/apache/netbeans/blob/master/platform/openide.filesystems/src/META-INF/upgrade/Repository.hint

[eirikbakke]

@mbien Didn't know about Jackpot rules! I did these edits manually, in particular since there were a lot of small variations in how loading was done or how simplifications could be made.

In any case I think this is the last batch of bulk changes to ImageIcon-related stuff.

[mbien]

Didn't know about Jackpot rules! I did these edits manually, in particular since there were a lot of small variations in how loading was done or how simplifications could be made.

yeah I didn't mention it earlier for that reason. But jackpot rules can be quite useful sometimes and I feel not enough know about them - so I make sure to point that out when the opportunity arises ;) They could be also put into the meta-inf of a module and used to help with migration third party code too.

For personal use, they can be put into conf/rules. I have a bunch of them active most of the time.

for example try putting this into a hint file in conf/rules and open profiler/profiler.heapwalker/src/org/netbeans/modules/profiler/heapwalk/model/BrowserUtils.java pre-refactoring:

new javax.swing.ImageIcon(org.openide.util.ImageUtilities.mergeImages($a.getImage(), $b.getImage(), $c, $d))
=>
org.openide.util.ImageUtilities.icon2ImageIcon(org.openide.util.ImageUtilities.mergeIcons($a, $b, $c, $d))
;;

[eirikbakke]

@mbien Oh, that's neat! I got that to work. Note that the folder in the home directory is called "config" rather than "conf", at least in my case. (Would also be useful in your readme to say "config/rules/" with a slash at the end to make it clear that rules is a directory rather than a single file.)

I know it's off-topic but there's one Jackpot rule I would love to have, which is similar to "Assign Return Value to New Variable", except using the return value as an iterable for a for loop. E.g. "Iterate over Expression in for Loop":

someIterableOfFooType

becomes

for (FooType fooType : someIterableOfFooType) {
}

Is that a rule you would know how to write off the top of your head? (Ideally it would work when FooType has generic parameters, too.)

[mbien]

Oh, that's neat! I got that to work. Note that the folder in the home directory is called "config" rather than "conf", at least in my case. (Would also be useful in your readme to say "config/rules/" with a slash at the end to make it clear that rules is a directory rather than a single file.)

thanks for pointing this out. will fix it.

"Iterate over Expression in for Loop"

this might be difficult to do with the hint files. But NB has a template which comes really close to what you want. type for + ctrl+space, then select the for loop you want, then paste the expression into the selected iterable field (or call a method etc) - this should update the rest.

[eirikbakke]

@mbien Oh, that works! I learned a new NetBeans feature today...

[eirikbakke]

I think this PR is now ready to merge, if you approve. (I already merged #8109 now, which was the first half of this work.)

[mbien]

pretty sure I reviewed this. Must have forgotten to hit approve. Thanks for the cleanups!

[eirikbakke]

Perfect, thank you! Merging.