Bug 53461

Created attachment 19716 [details]
Edited diff of the hack

*** Issue 52794 has been marked as a duplicate of this issue. ***

No need to look up .png variants, just rename your .png file to .gif
and keep current lookup mechanism.

Please NO! 
 
Don't just change the suffix on files to avoid having to change the code. 
Otherwise I would have to suggest to change all the *.java files into *.c;-) 
 
(And that suggestion is comming from a product which heavily depends on 
suffixes for it's own working:-() 
 
Thanks, 
-- Marco 

The proposed diff just makes things worse, I think. Prefer an API change.
Deprecate setIconBase and introduce something else, e.g.
setIconBaseWithExtension which would be almost the same but insert infixes
rather than suffixes.

(The suggestion to load icon paths from bundles may be unnecessary since you can
already brand icons loaded from AbstractNode or more generally from
Utilities.loadImage. You cannot change the extension but that probably won't
matter much - we should just replace all of our GIFs with PNGs in bulk anyway.)

DEFECT is not appropriate for this since you are not prevented from providing
PNG icons. You can return whatever Image you like from getIcon and
getOpenedIcon, which is the API. Rather, there is a convenience facility that
lets you write slightly less code (*) for AbstractNode subclasses, which works
with *.gif as documented. So this is an RFE to extend that convenience to other
file extensions.

(*) One method call rather than one or two 1-line method overrides.

The diff is just a way we solved that.
You can come up with a solution which is more suitable for NB, and which could
be more efficient (e.g. from performance point of view).

I'm going to implement this using the API change way.

OK, I'm asking for the API review of the change.

I'm adding a method AbstractNode.setIconBaseAndExtension(String base, String ext)
which sets up internal structures to support any extension of icon file.
The original method uses the default extension ".gif"
Without this change, one can't that easily use e.g. PNG icons

This change is in line with current Nodes architecture.

I'll attach the diff of changes including javadoc and api-changes entry.

Created attachment 22482 [details]
Proposed implementation

I would have a slight preference for

  setIconBaseWithExtension("org/foo/something.png");

over

  setIconBaseAndExtension("org/foo/something", ".png");

since the latter looks a bit artificial to me, and is less likely to give you a
search hit if you are grepping sources for usages of some icon. The change would
mean a little bit more work to implement: you need to scan the supplied string
for the last '/' (if any), then scan the suffix after the slash for the last '.'
(if any), which should be the infix insertion point. (Similar logic is used
widely, e.g. for the 'nbresloc' URL protocol handler.)

You could consider also deprecating the old setIconBase, and/or doing a bulk
replace of known sources to use the new method, but neither are necessary.

Can a test be written before integration? 

Finally implemented as suggested by Jesse. Available @since o-o-nodes 6.5
Extensive test writen and passing.

cvs/openide/nodes/src/org/openide/nodes/AbstractNode.java,v1.2