ImageUtilities code cleanup

[mbien]

• simplify if/else image loading cascade
• remove redundant logger with same name
• java 17 language level upgrade, dead code removal etc

to shrink my stash a bit :)

Originally I only intended to simplify the loading method and remove the logger - but there will be more eyes on this class due to all the usage refactoring so why not do a more complete cleanup. This might be easier to review in the IDE side-by-side view.

[eirikbakke]

Looks good, only question is whether we should also remove hashCode() from the two hashmap key classes that became records.

[mbien]

@eirikbakke btw while testing I noticed those log lines:

WARNING [org.openide.util.ImageUtilities]: loadImage(URI) called with unusual URI: nbres:/org/netbeans/modules/form/resources/palette/panel_16.png

seems to come from the form designer palette window.

the utility only accounts for nbresloc but not for nbres.

netbeans/platform/openide.util.ui/src/org/openide/util/ImageUtilities.java

Lines 234 to 243 in c384b9a

	if (scheme.equals("nbresloc")) { // NOI18N
	// Apply our dedicated handling logic. Omit the initial slash of the path.
	returnloadImage(uri.getPath().substring(1), false);
	} else {
	if (!(scheme.equals("file") ||
	scheme.equals("jar") && uri.toString().startsWith("jar:file:") ||
	scheme.equals("file")))
	{
	LOGGER.log(Level.WARNING, "loadImage(URI) called with unusual URI: {0}", uri);
	}

[mbien]

checked a few things and everything worked -> merging

thanks for review @eirikbakke!

[eirikbakke]

Nice catch regarding nbresloc vs. nbres! We should handle both. There is an explanation of the difference here:
https://netbeans.apache.org/wiki/main/netbeansdevelopperfaq/DevFaqUriVsUrl/

Apparently, nbresloc should use localization while nbres should not. So they should be handled the same except with a different second argument to loadImage(String,boolean).

Perhaps you could just add this as part of this PR? Maybe also update the Javadoc of loadImage(URI) to refer to say "nbres or nbresloc protocols" instead of just "nbresloc".

[mbien]

@eirikbakke oops I didn't see your message before merging. Lets add this in a separate PR to not mix cleanup with other changes.

[eirikbakke]

Yeah, sure. I can make the PR and request your review.

[mbien]

• simplify if/else image loading cascade
• remove redundant logger with same name
• java 17 language level upgrade, dead code removal etc

to shrink my stash a bit :)

Originally I only intended to simplify the loading method and remove the logger - but there will be more eyes on this class due to all the usage refactoring so why not do a more complete cleanup. This might be easier to review in the IDE side-by-side view.

[eirikbakke]

Looks good, only question is whether we should also remove hashCode() from the two hashmap key classes that became records.

[mbien]

@eirikbakke btw while testing I noticed those log lines:

WARNING [org.openide.util.ImageUtilities]: loadImage(URI) called with unusual URI: nbres:/org/netbeans/modules/form/resources/palette/panel_16.png

seems to come from the form designer palette window.

the utility only accounts for nbresloc but not for nbres.

netbeans/platform/openide.util.ui/src/org/openide/util/ImageUtilities.java

Lines 234 to 243 in c384b9a

	if (scheme.equals("nbresloc")) { // NOI18N
	// Apply our dedicated handling logic. Omit the initial slash of the path.
	returnloadImage(uri.getPath().substring(1), false);
	} else {
	if (!(scheme.equals("file") ||
	scheme.equals("jar") && uri.toString().startsWith("jar:file:") ||
	scheme.equals("file")))
	{
	LOGGER.log(Level.WARNING, "loadImage(URI) called with unusual URI: {0}", uri);
	}

[mbien]

checked a few things and everything worked -> merging

thanks for review @eirikbakke!

[eirikbakke]

Nice catch regarding nbresloc vs. nbres! We should handle both. There is an explanation of the difference here:
https://netbeans.apache.org/wiki/main/netbeansdevelopperfaq/DevFaqUriVsUrl/

Apparently, nbresloc should use localization while nbres should not. So they should be handled the same except with a different second argument to loadImage(String,boolean).

Perhaps you could just add this as part of this PR? Maybe also update the Javadoc of loadImage(URI) to refer to say "nbres or nbresloc protocols" instead of just "nbresloc".

[mbien]

@eirikbakke oops I didn't see your message before merging. Lets add this in a separate PR to not mix cleanup with other changes.

[eirikbakke]

Yeah, sure. I can make the PR and request your review.

[mbien]

• simplify if/else image loading cascade
• remove redundant logger with same name
• java 17 language level upgrade, dead code removal etc

to shrink my stash a bit :)

Originally I only intended to simplify the loading method and remove the logger - but there will be more eyes on this class due to all the usage refactoring so why not do a more complete cleanup. This might be easier to review in the IDE side-by-side view.

[eirikbakke]

Looks good, only question is whether we should also remove hashCode() from the two hashmap key classes that became records.

[mbien]

@eirikbakke btw while testing I noticed those log lines:

WARNING [org.openide.util.ImageUtilities]: loadImage(URI) called with unusual URI: nbres:/org/netbeans/modules/form/resources/palette/panel_16.png

seems to come from the form designer palette window.

the utility only accounts for nbresloc but not for nbres.

netbeans/platform/openide.util.ui/src/org/openide/util/ImageUtilities.java

Lines 234 to 243 in c384b9a

	if (scheme.equals("nbresloc")) { // NOI18N
	// Apply our dedicated handling logic. Omit the initial slash of the path.
	returnloadImage(uri.getPath().substring(1), false);
	} else {
	if (!(scheme.equals("file") ||
	scheme.equals("jar") && uri.toString().startsWith("jar:file:") ||
	scheme.equals("file")))
	{
	LOGGER.log(Level.WARNING, "loadImage(URI) called with unusual URI: {0}", uri);
	}

[mbien]

checked a few things and everything worked -> merging

thanks for review @eirikbakke!

[eirikbakke]

Nice catch regarding nbresloc vs. nbres! We should handle both. There is an explanation of the difference here:
https://netbeans.apache.org/wiki/main/netbeansdevelopperfaq/DevFaqUriVsUrl/

Apparently, nbresloc should use localization while nbres should not. So they should be handled the same except with a different second argument to loadImage(String,boolean).

Perhaps you could just add this as part of this PR? Maybe also update the Javadoc of loadImage(URI) to refer to say "nbres or nbresloc protocols" instead of just "nbresloc".

[mbien]

@eirikbakke oops I didn't see your message before merging. Lets add this in a separate PR to not mix cleanup with other changes.

[eirikbakke]

Yeah, sure. I can make the PR and request your review.

[mbien]

• simplify if/else image loading cascade
• remove redundant logger with same name
• java 17 language level upgrade, dead code removal etc

to shrink my stash a bit :)

Originally I only intended to simplify the loading method and remove the logger - but there will be more eyes on this class due to all the usage refactoring so why not do a more complete cleanup. This might be easier to review in the IDE side-by-side view.

[eirikbakke]

Looks good, only question is whether we should also remove hashCode() from the two hashmap key classes that became records.

[mbien]

@eirikbakke btw while testing I noticed those log lines:

WARNING [org.openide.util.ImageUtilities]: loadImage(URI) called with unusual URI: nbres:/org/netbeans/modules/form/resources/palette/panel_16.png

seems to come from the form designer palette window.

the utility only accounts for nbresloc but not for nbres.

netbeans/platform/openide.util.ui/src/org/openide/util/ImageUtilities.java

Lines 234 to 243 in c384b9a

	if (scheme.equals("nbresloc")) { // NOI18N
	// Apply our dedicated handling logic. Omit the initial slash of the path.
	returnloadImage(uri.getPath().substring(1), false);
	} else {
	if (!(scheme.equals("file") ||
	scheme.equals("jar") && uri.toString().startsWith("jar:file:") ||
	scheme.equals("file")))
	{
	LOGGER.log(Level.WARNING, "loadImage(URI) called with unusual URI: {0}", uri);
	}

[mbien]

checked a few things and everything worked -> merging

thanks for review @eirikbakke!

[eirikbakke]

Nice catch regarding nbresloc vs. nbres! We should handle both. There is an explanation of the difference here:
https://netbeans.apache.org/wiki/main/netbeansdevelopperfaq/DevFaqUriVsUrl/

Apparently, nbresloc should use localization while nbres should not. So they should be handled the same except with a different second argument to loadImage(String,boolean).

Perhaps you could just add this as part of this PR? Maybe also update the Javadoc of loadImage(URI) to refer to say "nbres or nbresloc protocols" instead of just "nbresloc".

[mbien]

@eirikbakke oops I didn't see your message before merging. Lets add this in a separate PR to not mix cleanup with other changes.

[eirikbakke]

Yeah, sure. I can make the PR and request your review.

[mbien]

• simplify if/else image loading cascade
• remove redundant logger with same name
• java 17 language level upgrade, dead code removal etc

to shrink my stash a bit :)

Originally I only intended to simplify the loading method and remove the logger - but there will be more eyes on this class due to all the usage refactoring so why not do a more complete cleanup. This might be easier to review in the IDE side-by-side view.

[eirikbakke]

Looks good, only question is whether we should also remove hashCode() from the two hashmap key classes that became records.

[mbien]

@eirikbakke btw while testing I noticed those log lines:

WARNING [org.openide.util.ImageUtilities]: loadImage(URI) called with unusual URI: nbres:/org/netbeans/modules/form/resources/palette/panel_16.png

seems to come from the form designer palette window.

the utility only accounts for nbresloc but not for nbres.

netbeans/platform/openide.util.ui/src/org/openide/util/ImageUtilities.java

Lines 234 to 243 in c384b9a

	if (scheme.equals("nbresloc")) { // NOI18N
	// Apply our dedicated handling logic. Omit the initial slash of the path.
	returnloadImage(uri.getPath().substring(1), false);
	} else {
	if (!(scheme.equals("file") ||
	scheme.equals("jar") && uri.toString().startsWith("jar:file:") ||
	scheme.equals("file")))
	{
	LOGGER.log(Level.WARNING, "loadImage(URI) called with unusual URI: {0}", uri);
	}

[mbien]

checked a few things and everything worked -> merging

thanks for review @eirikbakke!

[eirikbakke]

Nice catch regarding nbresloc vs. nbres! We should handle both. There is an explanation of the difference here:
https://netbeans.apache.org/wiki/main/netbeansdevelopperfaq/DevFaqUriVsUrl/

Apparently, nbresloc should use localization while nbres should not. So they should be handled the same except with a different second argument to loadImage(String,boolean).

Perhaps you could just add this as part of this PR? Maybe also update the Javadoc of loadImage(URI) to refer to say "nbres or nbresloc protocols" instead of just "nbresloc".

[mbien]

@eirikbakke oops I didn't see your message before merging. Lets add this in a separate PR to not mix cleanup with other changes.

[eirikbakke]

Yeah, sure. I can make the PR and request your review.

[mbien]

• simplify if/else image loading cascade
• remove redundant logger with same name
• java 17 language level upgrade, dead code removal etc

to shrink my stash a bit :)

Originally I only intended to simplify the loading method and remove the logger - but there will be more eyes on this class due to all the usage refactoring so why not do a more complete cleanup. This might be easier to review in the IDE side-by-side view.

[eirikbakke]

Looks good, only question is whether we should also remove hashCode() from the two hashmap key classes that became records.

[mbien]

@eirikbakke btw while testing I noticed those log lines:

WARNING [org.openide.util.ImageUtilities]: loadImage(URI) called with unusual URI: nbres:/org/netbeans/modules/form/resources/palette/panel_16.png

seems to come from the form designer palette window.

the utility only accounts for nbresloc but not for nbres.

netbeans/platform/openide.util.ui/src/org/openide/util/ImageUtilities.java

Lines 234 to 243 in c384b9a

	if (scheme.equals("nbresloc")) { // NOI18N
	// Apply our dedicated handling logic. Omit the initial slash of the path.
	returnloadImage(uri.getPath().substring(1), false);
	} else {
	if (!(scheme.equals("file") ||
	scheme.equals("jar") && uri.toString().startsWith("jar:file:") ||
	scheme.equals("file")))
	{
	LOGGER.log(Level.WARNING, "loadImage(URI) called with unusual URI: {0}", uri);
	}

[mbien]

checked a few things and everything worked -> merging

thanks for review @eirikbakke!

[eirikbakke]

Nice catch regarding nbresloc vs. nbres! We should handle both. There is an explanation of the difference here:
https://netbeans.apache.org/wiki/main/netbeansdevelopperfaq/DevFaqUriVsUrl/

Apparently, nbresloc should use localization while nbres should not. So they should be handled the same except with a different second argument to loadImage(String,boolean).

Perhaps you could just add this as part of this PR? Maybe also update the Javadoc of loadImage(URI) to refer to say "nbres or nbresloc protocols" instead of just "nbresloc".

[mbien]

@eirikbakke oops I didn't see your message before merging. Lets add this in a separate PR to not mix cleanup with other changes.

[eirikbakke]

Yeah, sure. I can make the PR and request your review.

[mbien]

• simplify if/else image loading cascade
• remove redundant logger with same name
• java 17 language level upgrade, dead code removal etc

to shrink my stash a bit :)

Originally I only intended to simplify the loading method and remove the logger - but there will be more eyes on this class due to all the usage refactoring so why not do a more complete cleanup. This might be easier to review in the IDE side-by-side view.

[eirikbakke]

Looks good, only question is whether we should also remove hashCode() from the two hashmap key classes that became records.

[mbien]

@eirikbakke btw while testing I noticed those log lines:

WARNING [org.openide.util.ImageUtilities]: loadImage(URI) called with unusual URI: nbres:/org/netbeans/modules/form/resources/palette/panel_16.png

seems to come from the form designer palette window.

the utility only accounts for nbresloc but not for nbres.

netbeans/platform/openide.util.ui/src/org/openide/util/ImageUtilities.java

Lines 234 to 243 in c384b9a

	if (scheme.equals("nbresloc")) { // NOI18N
	// Apply our dedicated handling logic. Omit the initial slash of the path.
	returnloadImage(uri.getPath().substring(1), false);
	} else {
	if (!(scheme.equals("file") ||
	scheme.equals("jar") && uri.toString().startsWith("jar:file:") ||
	scheme.equals("file")))
	{
	LOGGER.log(Level.WARNING, "loadImage(URI) called with unusual URI: {0}", uri);
	}

[mbien]

checked a few things and everything worked -> merging

thanks for review @eirikbakke!

[eirikbakke]

Nice catch regarding nbresloc vs. nbres! We should handle both. There is an explanation of the difference here:
https://netbeans.apache.org/wiki/main/netbeansdevelopperfaq/DevFaqUriVsUrl/

Apparently, nbresloc should use localization while nbres should not. So they should be handled the same except with a different second argument to loadImage(String,boolean).

Perhaps you could just add this as part of this PR? Maybe also update the Javadoc of loadImage(URI) to refer to say "nbres or nbresloc protocols" instead of just "nbresloc".

[mbien]

@eirikbakke oops I didn't see your message before merging. Lets add this in a separate PR to not mix cleanup with other changes.

[eirikbakke]

Yeah, sure. I can make the PR and request your review.

[mbien]

• simplify if/else image loading cascade
• remove redundant logger with same name
• java 17 language level upgrade, dead code removal etc

to shrink my stash a bit :)

Originally I only intended to simplify the loading method and remove the logger - but there will be more eyes on this class due to all the usage refactoring so why not do a more complete cleanup. This might be easier to review in the IDE side-by-side view.

[eirikbakke]

Looks good, only question is whether we should also remove hashCode() from the two hashmap key classes that became records.

[mbien]

@eirikbakke btw while testing I noticed those log lines:

WARNING [org.openide.util.ImageUtilities]: loadImage(URI) called with unusual URI: nbres:/org/netbeans/modules/form/resources/palette/panel_16.png

seems to come from the form designer palette window.

the utility only accounts for nbresloc but not for nbres.

netbeans/platform/openide.util.ui/src/org/openide/util/ImageUtilities.java

Lines 234 to 243 in c384b9a

	if (scheme.equals("nbresloc")) { // NOI18N
	// Apply our dedicated handling logic. Omit the initial slash of the path.
	returnloadImage(uri.getPath().substring(1), false);
	} else {
	if (!(scheme.equals("file") ||
	scheme.equals("jar") && uri.toString().startsWith("jar:file:") ||
	scheme.equals("file")))
	{
	LOGGER.log(Level.WARNING, "loadImage(URI) called with unusual URI: {0}", uri);
	}

[mbien]

checked a few things and everything worked -> merging

thanks for review @eirikbakke!

[eirikbakke]

Nice catch regarding nbresloc vs. nbres! We should handle both. There is an explanation of the difference here:
https://netbeans.apache.org/wiki/main/netbeansdevelopperfaq/DevFaqUriVsUrl/

Apparently, nbresloc should use localization while nbres should not. So they should be handled the same except with a different second argument to loadImage(String,boolean).

Perhaps you could just add this as part of this PR? Maybe also update the Javadoc of loadImage(URI) to refer to say "nbres or nbresloc protocols" instead of just "nbresloc".

[mbien]

@eirikbakke oops I didn't see your message before merging. Lets add this in a separate PR to not mix cleanup with other changes.

[eirikbakke]

Yeah, sure. I can make the PR and request your review.