Skip to content

Add format BibTeX source in JabRef style in import gui (continuing)#13591

Closed
NewCodes7 wants to merge 2 commits intoJabRef:mainfrom
NewCodes7:issue-13015
Closed

Add format BibTeX source in JabRef style in import gui (continuing)#13591
NewCodes7 wants to merge 2 commits intoJabRef:mainfrom
NewCodes7:issue-13015

Conversation

@NewCodes7
Copy link
Copy Markdown

@NewCodes7 NewCodes7 commented Jul 27, 2025

Closes #13015

I'm still working on it at the moment. I just posted PR quickly first.

Closes _____

Steps to test

Mandatory checks

  • I own the copyright of the code submitted and I license it under the MIT license
  • [.] Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
  • [.] Tests created for changes (if applicable)
  • [.] Manually tested changed features in running JabRef (always required)
  • [.] Screenshots added in PR description (if change is visible to the user)
  • [.] Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • [.] Checked 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 to the documentation repository.

@jabref-machine
Copy link
Copy Markdown
Collaborator

Note that your PR will not be reviewed/accepted until you have gone through the mandatory checks in the description and marked each of them them exactly in the format of [x] (done), [ ] (not done yet) or [/] (not applicable).

if (!viewModel.hasDuplicate(entry)) {
entriesListView.getCheckModel().check(entry);
displayBibTeX(entry, viewModel.getSourceString(entry));
displayBibTeX(entry, viewModel.getSourceString(entry, true));
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 boolean parameter true is a magic value that makes the code less maintainable and readable. Consider creating an enum or named constant to clarify the parameter's purpose.

@jabref-machine
Copy link
Copy Markdown
Collaborator

JUnit tests of jablib are failing. You can see which checks are failing by locating the box "Some checks were not successful" on the pull request page. To see the test output, locate "Tests / Unit tests (pull_request)" and click on it.

You can then run these tests in IntelliJ to reproduce the failing tests locally. We offer a quick test running howto in the section Final build system checks in our setup guide.

try {
new BibEntryWriter(fieldWriter, entryTypesManager).write(entry, bibWriter, selectedDb.getValue().getMode());
serializedStrings = new BibEntryWriter(fieldWriter, entryTypesManager).serializeAll(entries, selectedDb.getValue().getMode());
} catch (IOException ioException) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Empty string is returned on IOException without logging or proper error handling. This silently fails and may hide important issues from users and developers.

@HoussemNasri
Copy link
Copy Markdown
Member

I'm still working on it at the moment. I just posted PR quickly first.

Nice! We always encourage contributors to share code changes early. I'll go ahead and convert this PR to a draft so it's clear it's still in progress. Feel free to mark it as "Ready for Review" once it's good to go

@HoussemNasri HoussemNasri marked this pull request as draft July 27, 2025 16:36
@koppor
Copy link
Copy Markdown
Member

koppor commented Aug 24, 2025

I'm still working on it at the moment. I just posted PR quickly first.

May I ask about a timeline? We are trying to reduce the number of opened pull request - and there was no activity since one month here.

@koppor
Copy link
Copy Markdown
Member

koppor commented Aug 28, 2025

The solution at #13761 is much more simple and works.

I think, I did not overlook something.

Sometimes, its just small changes with a huge impact to the user.

@koppor koppor closed this Aug 28, 2025
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.

Import. Format BibTeX source in JabRef style

4 participants