Skip to content

Refine code of Source Tab#14504

Merged
koppor merged 15 commits into
mainfrom
refine-code
Dec 5, 2025
Merged

Refine code of Source Tab#14504
koppor merged 15 commits into
mainfrom
refine-code

Conversation

@koppor

@koppor koppor commented Dec 3, 2025

Copy link
Copy Markdown
Member

Triggered by #14501 --> Adds org.jabref.model.entry.BibEntry#getStringRepresentation

Changed Exceptions in source tab to notify

Steps to test

Use source tab of entry editor

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.

@Siedlerchr

Copy link
Copy Markdown
Member

screenshot please

@Siedlerchr Siedlerchr closed this Dec 3, 2025
@Siedlerchr Siedlerchr reopened this Dec 3, 2025
}

private void highlightField(Optional<Field> optionalField, List<String> terms) {
private void buildPatternAndHighlightField(Optional<Field> optionalField, List<String> terms) {

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think, this is not right, because after calling buildSeardchPattern, the code is used in the subsequent lies to call highlightField.

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.

I just extracted the method, no semantic change and renamed it now for clarity

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Ah, I should read the complete method name ^^

@koppor

koppor commented Dec 3, 2025

Copy link
Copy Markdown
Member Author

Maybe needs even more work xyz as text leads to exception

grafik

@koppor

koppor commented Dec 3, 2025

Copy link
Copy Markdown
Member Author
grafik

@Siedlerchr

Copy link
Copy Markdown
Member

I am not sure if notify is a good idea here as it will vanish and you miss the error when you don't lool

@koppor koppor changed the title Use .notify instead of Exception in Source Tab Refine code of Source Tab Dec 3, 2025
@koppor

koppor commented Dec 3, 2025

Copy link
Copy Markdown
Member Author

I am not sure if notify is a good idea here as it will vanish and you miss the error when you don't lool

Brought it back. Not using Exceptions for control flow, but explicit calls to the validation things.

@koppor koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Dec 4, 2025
@koppor koppor mentioned this pull request Dec 5, 2025
@koppor koppor added this pull request to the merge queue Dec 5, 2025
Merged via the queue into main with commit 0b2baaa Dec 5, 2025
59 of 66 checks passed
@koppor koppor deleted the refine-code branch December 5, 2025 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants