Skip to content

Fix BibTeX "twin" at citation relations#12124

Merged
calixtus merged 13 commits intomainfrom
fix-citation-relations
Oct 30, 2024
Merged

Fix BibTeX "twin" at citation relations#12124
calixtus merged 13 commits intomainfrom
fix-citation-relations

Conversation

@koppor
Copy link
Member

@koppor koppor commented Oct 29, 2024

Follow-ujp to #12123

  • Add getter+setter for "cites" to BibEntry
  • Refactor CitationsRelationsTabViewModel
  • Group citation keys together in JabRef_en.properties
  • We fixed an issue where the import of an entry cited by an entry caused a change of the citation key of the citing entry.

I think, CHANGELOG.md entry too special, thus not adding.

Mandatory checks

  • I own the copyright of the code submitted and I licence 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 (for UI changes)
  • 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.

- Add getter+setter for "cites" to BibEntry
- Refactor CitationsRelationsTabViewModel
- Group citation keys together in JabRef_en.properties
- We fixed an issue where the import of an entry cited by an entry caused a change of the citation key of the citing entry.
task.onSuccess(parserResult -> {
tabContainer.addTab(parserResult.getDatabaseContext(), true);
dialogService.notify(Localization.lang("Imported entries") + ": " + parserResult.getDatabase().getEntries().size());
dialogService.notify(Localization.lang("%0 entry(ies) imported", parserResult.getDatabase().getEntries().size()));
Copy link
Member

Choose a reason for hiding this comment

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

This reads weird. We should make this consistent with plural check if > 1

Copy link
Member Author

Choose a reason for hiding this comment

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

First introduced at #10732. Long call w/ @calixtus We never wrote it down as follow-up.

TL;DR: It’s not just singular and plural

  • zero → “لم نزرع أي شجرة حتى الآن”
  • one → “لقد زرعنا شجرة ١ حتى الآن”
  • two → “لقد زرعنا شجرتين ٢ حتى الآن”
  • few → “لقد زرعنا ٣ شجرات حتى الآن”
  • many → “لقد زرعنا ١١ شجرة حتى الآن”
  • other → “لقد زرعنا ١٠٠ شجرة حتى الآن”

Longer: https://phrase.com/blog/posts/pluralization/

We decided to remove singular/plural completely and translate only one thing.

@HoussemNasri Seeing #9243, WDYT?

Copy link
Member

Choose a reason for hiding this comment

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

Should we write an ADR?

Copy link
Member

@HoussemNasri HoussemNasri Oct 30, 2024

Choose a reason for hiding this comment

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

It would be great if the translation was accurate, but I think it should be fine. For example, both of the following variations are understandable for anyone who reads Arabic. I'm not sure about grammatical correctness, however.

zero → “لقد زرعنا 0 شجرة حتى الآن”
one → “لقد زرعنا ١ شجرة حتى الآن”

Or we could go with something like "Number of entries imported: 3," but that's not necessary, in my opinion.

*/
public SequencedSet<String> getCites() {
return this.getField(StandardField.CITES)
.map(content -> Arrays.stream(content.split(",")))
Copy link
Member

Choose a reason for hiding this comment

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

can't you use flatmap with filter is Present check

Copy link
Member Author

@koppor koppor Oct 29, 2024

Choose a reason for hiding this comment

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

Tried, was very red:

image

or

image


Alternative would be

        Arrays.stream(this.getField(StandardField.CITES).orElse("").split(","))
              .map(String::trim)
              .filter(key -> !key.isEmpty())
              .collect(Collectors.toCollection(LinkedHashSet::new));

but I don't like the orElse(""), because this is hacky, isn't it?

@koppor koppor marked this pull request as draft October 29, 2024 23:35
@koppor koppor mentioned this pull request Oct 30, 2024
7 tasks
@koppor koppor marked this pull request as ready for review October 30, 2024 11:33
# Conflicts:
#	src/main/java/org/jabref/gui/LibraryTab.java
@koppor koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Oct 30, 2024
@github-actions
Copy link
Contributor

github-actions bot commented Oct 30, 2024

The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build.

@calixtus calixtus added this pull request to the merge queue Oct 30, 2024
Merged via the queue into main with commit a2a64f1 Oct 30, 2024
@calixtus calixtus deleted the fix-citation-relations branch October 30, 2024 17:52
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.

4 participants