Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Enhance customization of entry types #11230

Draft
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

aqurilla
Copy link
Contributor

This fixes #9840 by adding a new data format for custom entry types that supports saving multiple field properties

Mandatory checks

  • Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
  • 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.

@aqurilla
Copy link
Contributor Author

Hi! Should org.jabref.logic.exporter.MetaDataSerializer#serializeCustomEntryTypes be updated so that custom entry types are always serialized to the v2 format going forward?

@koppor
Copy link
Member

koppor commented Apr 21, 2024

Hi! Should org.jabref.logic.exporter.MetaDataSerializer#serializeCustomEntryTypes be updated so that custom entry types are always serialized to the v2 format going forward?

Meta: I miss what you would do :p

You know, I am thinking in MADR, especially using "decision drivers".

Decicion drivers:

  • Maintainable solution (code, releases)
  • Backward compatibility if possible (some users do not switch JabRef versions - or use two versions in parallel, because newer versions sometimes do not work)

I would vote to always use v2 here - but then include the feature in the next major release (because this breaks compatibility). However, I do not see JabRef 6.0 coming the next weeks.

Thus, use (existing) v1 serialization if single field properties and use v2 if v1 cannot be used. -- For JabRef 7.0 (!) we can remove that logic and always migrate to v2.

@aqurilla
Copy link
Contributor Author

Same here! 😄

Thankyou for the pointers!
I think we would have to use v2 in all cases where a custom FieldProperty is specified, even if it is just a single field property?

For example, using the person entry type with Name field having a single field property PERSON_NAMES.
Using v1 we would get: jabref-entrytype: person: req[Name] opt[]

Using v2 would retain the field property info: jabref-entrytype-v2: person: req[Name|PERSON_NAMES] opt[]

@koppor
Copy link
Member

koppor commented Apr 21, 2024

I think we would have to use v2 in all cases where a custom FieldProperty is specified, even if it is just a single field property?

I fully agree!

What could work is mixing v1 and v2? Only use v2 for those entry types requiring it and write the others as v1. Thus, not all are "lost" when opening with an old version, but only "some".

Thinking aloud: We should not do, because of too many changes in the bib file: write v1 always and v2 *additionally. The reader then needs to prioritize v2 over v1. We could keep that for version 6.0 and remove that for version 6.1.

@aqurilla
Copy link
Contributor Author

Thinking aloud: We should not do, because of too many changes in the bib file: write v1 always and v2 *additionally. The reader then needs to prioritize v2 over v1. We could keep that for version 6.0 and remove that for version 6.1.

yes that sounds good!
we can write both v1 and v2, and the reader can prioritize v2 if present

@aqurilla
Copy link
Contributor Author

aqurilla commented May 4, 2024

Hi @koppor, the parser has been updated so that org.jabref.logic.importer.util.MetaDataParser#parseCustomEntryType can parse both v1 and v2 entry types.

Regarding backward compatibility, it looks like even if we write both v1 and v2 it can lead to an incorrect additional entry type when reading using an older version, because the v2 entry type would be parsed incorrectly. Given this, I was wondering if we can write only the v2 version in the Serializer?

@koppor
Copy link
Member

koppor commented May 5, 2024

Regarding backward compatibility, it looks like even if we write both v1 and v2 it can lead to an incorrect additional entry type when reading using an older version, because the v2 entry type would be parsed incorrectly. Given this, I was wondering if we can write only the v2 version in the Serializer?

Without reading the code, I don't understand. Isn't v2 just ignored in older versions? I thought, unknown meta data keys would be ignored?

Please start to craft an ADR (https://adr.github.io/madr/) to capture pros and cons of each option.

@koppor
Copy link
Member

koppor commented Jun 3, 2024

End user requirement: Support of "cited by". See https://discourse.jabref.org/t/wrapping-of-custom-entry-fields/4339.

@aqurilla
Copy link
Contributor Author

aqurilla commented Jun 6, 2024

Thanks! I’ll look into support for that too

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
The tool reviewdog already placed comments on GitHub to indicate the places. See the tab "Files" in you PR.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

You can check review dog's comments at the tab "Files changed" of your pull request.

@HoussemNasri
Copy link
Member

Hello @aqurilla, any plans to continue working on this? I think it would be a huge waste to not merge this valuable contribution.

@aqurilla
Copy link
Contributor Author

Hi @HoussemNasri, yes I’ll continue on this. Got a bit sidetracked!

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.

Enhance customization of entry types
3 participants