Skip to content

Fix: No exception if file is not within a git repository#13839

Merged
calixtus merged 7 commits into
mainfrom
fix-not-under-version-control
Sep 8, 2025
Merged

Fix: No exception if file is not within a git repository#13839
calixtus merged 7 commits into
mainfrom
fix-not-under-version-control

Conversation

@koppor

@koppor koppor commented Sep 8, 2025

Copy link
Copy Markdown
Member

Steps to test

  1. Open several bib files
  2. Switch tabs back and forth
  3. No exception should be shown.

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.

koppor and others added 4 commits September 8, 2025 20:54
@koppor
Rewrite public static GitStatusSnapshot checkStatus(Path anyPathInsid…
ca2c34c 
…eRepo) {
@koppor
Reuse "handler"
842e39a
@koppor @calixtus
Inject GitHandlerRegistry
bd21f65 
Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com>
@koppor
Get rid of exception
0c7f415
@koppor koppor marked this pull request as draft September 8, 2025 19:29
trag-bot[bot]
trag-bot Bot reviewed Sep 8, 2025
View reviewed changes
Comment thread jabgui/src/main/java/org/jabref/gui/actions/ActionHelper.java
@koppor @calixtus
Use constructor-based dependency injection
ffd51c5 
Co-authored-by: Carl Christian Snethlage <50491877+calixtus@users.noreply.github.com>
@koppor koppor marked this pull request as ready for review September 8, 2025 19:59
@koppor koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Sep 8, 2025
@trag-bot

trag-bot Bot commented Sep 8, 2025

Copy link
Copy Markdown

@trag-bot didn't find any issues in the code! ✅✨

@koppor koppor changed the title Remove exception if file is not on a repository Remove exception if file is not within a repository Sep 8, 2025
@koppor koppor changed the title Remove exception if file is not within a repository Remove exception if file is not within a git repository Sep 8, 2025
@koppor koppor changed the title Remove exception if file is not within a git repository Fix: Nexception if file is not within a git repository Sep 8, 2025
@koppor koppor changed the title Fix: Nexception if file is not within a git repository Fix: No exception if file is not within a git repository Sep 8, 2025
@trag-bot

trag-bot Bot commented Sep 8, 2025

Copy link
Copy Markdown

@trag-bot didn't find any issues in the code! ✅✨

@calixtus calixtus enabled auto-merge September 8, 2025 20:09
trag-bot[bot]
trag-bot Bot reviewed Sep 8, 2025
View reviewed changes
Comment thread jabgui/src/main/java/org/jabref/gui/git/GitShareToGitHubDialogView.java
Comment on lines +53 to +54
@Inject
private GitHandlerRegistry gitHandlerRegistry;

@trag-bot trag-bot Bot Sep 8, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Field injection should be avoided in favor of constructor injection to ensure proper initialization and make dependencies explicit. This also improves testability and makes the class immutable.

trag-bot[bot]
trag-bot Bot reviewed Sep 8, 2025
View reviewed changes
Comment thread jabgui/src/main/java/org/jabref/gui/git/GitShareToGitHubDialogView.java
private TaskExecutor taskExecutor;

@Inject
private GitHandlerRegistry gitHandlerRegistry;

@trag-bot trag-bot Bot Sep 8, 2025

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The field should be marked as final since it's injected in the constructor and never reassigned, following the principles of immutability from Effective Java.

@calixtus calixtus added this pull request to the merge queue Sep 8, 2025
Merged via the queue into main with commit 87a8f42 Sep 8, 2025
40 checks passed
@calixtus calixtus deleted the fix-not-under-version-control branch September 8, 2025 20:32
Siedlerchr added a commit that referenced this pull request Sep 9, 2025
@Siedlerchr
Merge remote-tracking branch 'upstream/main'
511549f 
* upstream/main:
  Fix typo in PR template
  Fix: No exception if file is not within a git repository (#13839)
  Fix lsp4j modules (#13835)
  New Crowdin updates (#13837)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@trag-bot trag-bot[bot] trag-bot[bot] left review comments

@calixtus calixtus calixtus approved these changes

Assignees

No one assigned

Labels

status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@koppor @calixtus