Skip to content

Add robust log level handling in JVSGLoggerLogManager#3287

Merged
HeikoKlare merged 1 commit intoeclipse-platform:masterfrom
HeikoKlare:jsvglogger-level-config
May 7, 2026
Merged

Add robust log level handling in JVSGLoggerLogManager#3287
HeikoKlare merged 1 commit intoeclipse-platform:masterfrom
HeikoKlare:jsvglogger-level-config

Conversation

@HeikoKlare
Copy link
Copy Markdown
Contributor

The JVSGLoggerLogManager allows to customize the log level with a system property. When an invalid log level is passed, the call currently throws an exception and makes the whole rasterizer initialization fail, leading to the non-availability of an SVG rasterizer throughout the whole application lifecycle.

This change makes the system property processing more robust: when an invalid level is used, an error is printed and the implementation falls back to using "ERROR" as default log level. In addition, capitalization is ignored by always making the passed value upper case. As a result of this change, it not be possible to make the rasterizer implementation fail with an invalid system property anymore.

Resolves #3283 (comment)

@HeikoKlare HeikoKlare requested a review from merks May 6, 2026 19:14
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 6, 2026

Test Results

  182 files  ±0    182 suites  ±0   26m 58s ⏱️ + 1m 23s
4 723 tests ±0  4 700 ✅ ±0   23 💤 ±0  0 ❌ ±0 
6 812 runs  ±0  6 649 ✅ ±0  163 💤 ±0  0 ❌ ±0 

Results for commit 4748451. ± Comparison against base commit c1fd7a0.

♻️ This comment has been updated with latest results.

Copy link
Copy Markdown
Contributor

@merks merks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks better. Just a tiny name consistency issue.


import com.github.weisj.jsvg.logging.LogManager;
import com.github.weisj.jsvg.logging.Logger;
import com.github.weisj.jsvg.logging.Logger.Level;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe use the Logger.Level like the other parts to avoid this import? Or make the rest consistent?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, makes sense. Logger.Level is better to read than a generic Level, so I made the change consistent with how it was used alreday.

The JVSGLoggerLogManager allows to customize the log level with a system
property. When an invalid log level is passed, the call currently throws
an exception and makes the whole rasterizer initialization fail, leading
to the non-availability of an SVG rasterizer throughout the whole
application lifecycle.

This change makes the system property processing more robust: when an
invalid level is used, an error is printed and the implementation falls
back to using "ERROR" as default log level. In addition, capitalization
is ignored by always making the passed value upper case. As a result of
this change, it not be possible to make the rasterizer implementation
fail with an invalid system property anymore.
@HeikoKlare HeikoKlare force-pushed the jsvglogger-level-config branch from dee73cc to 4748451 Compare May 7, 2026 09:48
@HeikoKlare HeikoKlare merged commit d63140f into eclipse-platform:master May 7, 2026
24 checks passed
@HeikoKlare HeikoKlare deleted the jsvglogger-level-config branch May 7, 2026 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants