Skip to content

Conversation

@subhramit
Copy link
Member

@subhramit subhramit commented Sep 13, 2025

Sisyphus says hi again

Follow-up to #13872

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.

Level logLevel = Arrays.stream(args).anyMatch("--debug"::equalsIgnoreCase)
? Level.DEBUG
: Level.INFO;
Level logLevel =
Copy link
Member

Choose a reason for hiding this comment

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

Can we move this to one line?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done manually in 3aa77ac

Copy link
Member

Choose a reason for hiding this comment

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

WOuld not do this as a general rule. There are many locations - LIKE THIS ONE- where it should not be reduced to one line, because it is not easily readable anymore, gets to wide to the right etc.
I know exactly about this one, because i deliberately decided to do so here.

This was referenced Sep 13, 2025
@koppor koppor added the dev: code-quality Issues related to code or architecture decisions label Sep 13, 2025
# Conflicts:
#	jabgui/src/main/java/org/jabref/gui/edit/automaticfiededitor/MoveFieldValueAction.java
#	jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/cell/sidebuttons/ToggleMergeUnmergeButton.java
#	jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/CommentMerger.java
#	jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/FieldMerger.java
#	jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/FileMerger.java
#	jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/GroupMerger.java
#	jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/KeywordMerger.java
#	jablib/src/main/java/org/jabref/logic/layout/format/HTMLChars.java
#	jablib/src/main/java/org/jabref/model/entry/identifier/ArXivIdentifier.java
#	jablib/src/main/java/org/jabref/model/entry/identifier/EprintIdentifier.java
@koppor koppor added the status: awaiting-second-review For non-trivial changes label Sep 13, 2025
koppor
koppor previously approved these changes Sep 13, 2025
*
* @param dataLoadingTask The task to execute to load the data asynchronously.
* @param file the path to the file (loaded by the dataLoadingTask)
* @param file the path to the file (loaded by the dataLoadingTask)
Copy link
Member Author

Choose a reason for hiding this comment

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

This is a bit odd

Copy link
Member

Choose a reason for hiding this comment

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

Might be formatted to "createLibraryTab" in the next line?

Copy link
Member

Choose a reason for hiding this comment

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

IntelliJ style - description always on the same column

Comment on lines -373 to +372
* @param backupDir The backup directory
* @param backupDir The backup directory
Copy link
Member Author

Choose a reason for hiding this comment

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

This is relatively okay

@subhramit
Copy link
Member Author

Anybody objecting to this PR must do so right away or forever hold their silence.

@subhramit subhramit changed the title IntelliJ auto-format Reformat codebase (more carefully) Sep 13, 2025
@calixtus
Copy link
Member

I will check. Give me a few minutes

@koppor koppor mentioned this pull request Sep 13, 2025
3 tasks
@calixtus
Copy link
Member

calixtus commented Sep 13, 2025

grafik

Still feels off, but is no show stopper

@calixtus calixtus added this pull request to the merge queue Sep 13, 2025
@koppor
Copy link
Member

koppor commented Sep 13, 2025

grafik

Still feels off, but is no show stopper

The closing is below the dot, because it belongs to the select.... Better than if the formatter moved everything to the right 😅

Merged via the queue into JabRef:main with commit f3cfa49 Sep 13, 2025
50 checks passed
@calixtus calixtus deleted the reformat-again branch September 13, 2025 20:06
@koppor koppor mentioned this pull request Sep 13, 2025
1 task
alexsukhin pushed a commit to alexsukhin/jabref that referenced this pull request Sep 14, 2025
* Fix non record comments by carl

# Conflicts:
#	jabgui/src/main/java/org/jabref/gui/edit/automaticfiededitor/MoveFieldValueAction.java
#	jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/cell/sidebuttons/ToggleMergeUnmergeButton.java
#	jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/CommentMerger.java
#	jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/FieldMerger.java
#	jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/FileMerger.java
#	jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/GroupMerger.java
#	jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/KeywordMerger.java
#	jablib/src/main/java/org/jabref/logic/layout/format/HTMLChars.java
#	jablib/src/main/java/org/jabref/model/entry/identifier/ArXivIdentifier.java
#	jablib/src/main/java/org/jabref/model/entry/identifier/EprintIdentifier.java

* Add file exceptions

* Remove shebang line

* Remove shebang line

* Remove shebang line

* Expand variables & rename class

---------

Co-authored-by: Oliver Kopp <[email protected]>
Siedlerchr added a commit that referenced this pull request Sep 14, 2025
* upstream/main:
  Add hint on auto formatting (#13886)
  Add ADR-0050 on code formatter (#13895)
  Reformat codebase (more carefully) (#13885)
  New Crowdin updates (#13892)
  Fix record wrapping (#13889)
  Fix JavaDoc (#13888)
  Improve IntelliJ settings (#13887)
  Manual code reformattings (#13883)
@koppor koppor mentioned this pull request Sep 14, 2025
1 task
github-merge-queue bot pushed a commit that referenced this pull request Nov 11, 2025
* Add cleanup dialog tabs with individual tab preferences

* Fixed indentation and added commenting

* Fix Trag-bot review issues

- Removed trivial comments
- Renamed PDF-related variables
- Updated methods to return Optional
- Used Optional property for FieldFormatterCleanups

* Fix Trag-bot review issues

- Removed trivial comments
- Fixed CHANGELOG.md

* Fix Trag-bot review issues

- Removed trivial comments
- Improved Optional checks
- Perform null check for other parameters

* Avoid nested Optionals, returning Optional<CleanupPreferences> directly

* Refactor CleanupPreferences by keeping one assertion per test

* Converted tests to assertEquals

* Maintain consistent naming conventions

* Returns CleanupPreferences directly since value is always present

* Initial review refactor draft
- Create new ViewModel, pull logic from Action and SingleAction into ViewModel
- Move Apply button to each tab
- Remove categories from ENUM and keep enums of all jobs in each respective tab to be used for cleanup

* fix import error!

* Reformat codebase (more carefully) (#13885)

* Fix non record comments by carl

# Conflicts:
#	jabgui/src/main/java/org/jabref/gui/edit/automaticfiededitor/MoveFieldValueAction.java
#	jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/cell/sidebuttons/ToggleMergeUnmergeButton.java
#	jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/CommentMerger.java
#	jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/FieldMerger.java
#	jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/FileMerger.java
#	jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/GroupMerger.java
#	jabgui/src/main/java/org/jabref/gui/mergeentries/threewaymerge/fieldsmerger/KeywordMerger.java
#	jablib/src/main/java/org/jabref/logic/layout/format/HTMLChars.java
#	jablib/src/main/java/org/jabref/model/entry/identifier/ArXivIdentifier.java
#	jablib/src/main/java/org/jabref/model/entry/identifier/EprintIdentifier.java

* Add file exceptions

* Remove shebang line

* Remove shebang line

* Remove shebang line

* Expand variables & rename class

---------

Co-authored-by: Oliver Kopp <[email protected]>

* fix import error & merge

* Apply OpenRewrite Cleanup

* Refactor Cleanup Tabs
- Moved cleanup panel logic into CleanupDialogViewModel for better separation of UI and logic
- Changed tabSupplier and taskExecutor from Optional to nullable parameters
- Moved updateWith logic into the record for cleaner preference updates.
- General design improvements: more maintainable.

* Fix getDisplayName method

* Fix formatting

* Trag-bot review and fix en properties

* fix indentation plssss

* format properly and change to observablelist

* fix formatting entriestoprocess (please)

* Updated names and changed optional dependencies back to nullable

* Refactored panels to use separate ViewModels
- Introduced ViewModels to encapsulate state and logic for panels.
- Replaced direct UI manipulation with bidirectional bindings.
- Ensures cleaner UI logic, easier maintenance

* Moved ALL_JOBS to respective ViewModels, small naming changes

* Replaced requireNotNull to @NotNull following #13957

* Address review feedback in CleanupDialogViewModel

- Remove redundant comments following self-explanatory code
- Add modifiedEntriesCount > 0 condition
- Use "entry(s)" localization form for clean up message

---------

Co-authored-by: Subhramit Basu <[email protected]>
Co-authored-by: Oliver Kopp <[email protected]>
Co-authored-by: Christoph <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dev: code-quality Issues related to code or architecture decisions project: jabcon status: awaiting-second-review For non-trivial changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants