Skip to content

feat: implement no_color mode#14119

Merged
calixtus merged 18 commits into
JabRef:mainfrom
vitinh0z:feature/support-no-color
Oct 21, 2025
Merged

feat: implement no_color mode#14119
calixtus merged 18 commits into
JabRef:mainfrom
vitinh0z:feature/support-no-color

Conversation

@vitinh0z

@vitinh0z vitinh0z commented Oct 19, 2025

Copy link
Copy Markdown
Contributor

Closes #14086 - Support NO_COLOR environment variable

This PR implements the functionality for issue #14089 (support for the NO_COLOR standard).

The main code change was refactoring the JABREF_BANNER constant (in BuildInfo.java) to use the terminal-text-formatter library, which automatically handles the NO_COLOR environment variable. This also required correcting the 4 usage points (in jabgui and jabkit) to use the .concat() method.

The fixes I implemented (which are included in this PR) are:

  1. In jablib/build.gradle.kts: Changed the terminal-text-formatter dependency from implementation to api.
  2. In jablib/module-info.java: Added requires transitive textFormatter; to expose the module.
  3. In jabkit/module-info.java: Added requires textFormatter;.
  4. In build-logic/.../dependency-rules.gradle.kts: Added patchRealModule("io.github.darvil82.utils", "utils") to fix a module patching error related to the new dependency.

Steps to test

  1. In a terminal running in x64 emulation mode (using an x64 JDK), run .\gradlew.bat :jabkit:run.
  2. Expected Result: The JabRef banner should appear in full color.
  3. In the same terminal, set the NO_COLOR environment variable: $env:NO_COLOR="true" (for PowerShell).
  4. Run .\gradlew.bat :jabkit:run again.
  5. Expected Result: The JabRef banner should now appear as plain text, with no colors.

Mandatory checks

  • I own the copyright of the code submitted and I license it under the MIT license
  • I manually tested my changes in running JabRef (always required)
  • [/] I added JUnit tests for changes (if applicable)
  • I added screenshots in the PR description (if change is visible to the user)
  • [/] I described the change in CHANGELOG.md in a way that is understandable for the average user (if change is visible to the user)
  • [/] I checked the user documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request updating file(s) in https://github.com/JabRef/user-documentation/tree/main/en.

with NO_COLOR

comCor

Default

semCor

@github-actions

github-actions Bot commented Oct 19, 2025

Copy link
Copy Markdown
Contributor

Hey @vitinh0z!

Thank you for contributing to JabRef! Your help is truly appreciated ❤️.

We have automatic checks in place, based on which you will soon get automated feedback if any of them are failing. We also use TragBot with custom rules that scans your changes and provides some preliminary comments, before a maintainer takes a look. TragBot is still learning, and may not always be accurate. In the "Files changed" tab, you can go through its comments and just click on "Resolve conversation" if you are sure that it is incorrect, or comment on the conversation if you are doubtful.

Please re-check our contribution guide in case of any other doubts related to our contribution workflow.

koppor
koppor previously requested changes Oct 20, 2025
View reviewed changes

@koppor koppor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good first shot. A PR should modify as least places as possible. Why the changes var to val? Why the reformattings of the output part? Why the two more empty lines? I think, all of that can be undone?

@vitinh0z

vitinh0z commented Oct 20, 2025

Copy link
Copy Markdown
Contributor Author

I changed var to val because Gradle expects tasks to be immutable, which avoids caching and instrumentation issues (and is a good practice).

Making this change helped me get the project working in the terminal (I spent a week troubleshooting the build).

But I can revert the change.

About the empty spaces, I can revert them.

calixtus
calixtus requested changes Oct 20, 2025
View reviewed changes
Comment thread jabkit/build.gradle.kts Outdated
Comment thread jablib/build.gradle.kts Outdated
Comment thread jablib/build.gradle.kts Outdated
Comment thread jablib/build.gradle.kts Outdated
Comment thread jablib/build.gradle.kts Outdated
Comment thread jablib/build.gradle.kts Outdated
Comment thread jablib/build.gradle.kts Outdated
Comment thread jablib/src/main/java/org/jabref/logic/util/BuildInfo.java Outdated
@Siedlerchr

Siedlerchr commented Oct 20, 2025

Copy link
Copy Markdown
Member

@vitinh0z You can to the changes from var to val in a follow up/separate PR

@vitinh0z
Refactor: Apply suggestions from code review
a906245 
- Move textFormatter version to version catalog

- Reformat JABREF_BANNER concatenation

change val to var
@vitinh0z
fix format
d63cfc0
@vitinh0z

vitinh0z commented Oct 20, 2025

Copy link
Copy Markdown
Contributor Author

@calixtus
image

Is that right?

@calixtus

calixtus commented Oct 20, 2025

Copy link
Copy Markdown
Member

Is that right?

Yes

@vitinh0z

vitinh0z commented Oct 20, 2025

Copy link
Copy Markdown
Contributor Author

what is ?
image

@Siedlerchr

Siedlerchr commented Oct 20, 2025

Copy link
Copy Markdown
Member

a failing test. if youc clilck on it you see the details. probably some temporary issue

@vitinh0z

vitinh0z commented Oct 20, 2025

Copy link
Copy Markdown
Contributor Author

i think that is done

@vitinh0z vitinh0z requested review from calixtus and koppor October 21, 2025 12:05
@Siedlerchr

Siedlerchr commented Oct 21, 2025 •
edited
Loading

Copy link
Copy Markdown
Member

you don't have to always merge main, as long as github shows no conflict you can leave it. This otherwise creates too much noise as we always get a notificaiton

@vitinh0z

vitinh0z commented Oct 21, 2025

Copy link
Copy Markdown
Contributor Author

Ok ok

@calixtus

calixtus commented Oct 21, 2025

Copy link
Copy Markdown
Member

I experimented a bit with it, removed some unnecessary lines, moved the textformatter entierly to BuildInfo to reduce deps. Tested it, works. Should be good to go now.

@calixtus calixtus enabled auto-merge October 21, 2025 23:11
@calixtus calixtus added this pull request to the merge queue Oct 21, 2025
Merged via the queue into JabRef:main with commit bb7052b Oct 21, 2025
45 checks passed
@github-actions github-actions Bot mentioned this pull request Oct 21, 2025
Closed
merlinymy pushed a commit to merlinymy/jabref that referenced this pull request Nov 19, 2025
@vitinh0z @calixtus @merlinymy
feat: implement no_color mode (JabRef#14119)
b3bacbe 
* feat: implement no_color mode

* fix format

* apply OpenRewrite changes for String.format

* Refactor: Apply suggestions from code review

- Move textFormatter version to version catalog

- Reformat JABREF_BANNER concatenation

change val to var

* fix format

* fix format

* fix format

* fix (again) format

* Nitpicks and removed unnecessary changes

* Remove more unnecessary changes

* Undo gradle-wrapper.properties again

* Remove transitive, removed unnecessary deps

* Removed more unnecessary changes

* Reintroduce deps after testing

---------

Co-authored-by: Carl Christian Snethlage <calixtus@users.noreply.github.com>
@vitinh0z vitinh0z deleted the feature/support-no-color branch April 10, 2026 16:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@calixtus calixtus calixtus approved these changes

@koppor koppor Awaiting requested review from koppor

Assignees

No one assigned

Labels

first contrib

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Support NO_COLOR environment variable

4 participants

@vitinh0z @Siedlerchr @calixtus @koppor