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

Display deprecated fields in misc entry also in optional fields tab #4389

Closed
wants to merge 1 commit into from

Conversation

Siedlerchr
Copy link
Member

@Siedlerchr Siedlerchr commented Oct 20, 2018

Fixes #4388

The reason for the original code was to fix #3046, however I couldn't reproduce the issue mentioned there after I removed it. The custom entry types dialog effectively prevents you from adding from the same field twice.


  • Change in CHANGELOG.md described
  • Tests created for changes
  • Manually tested changed features in running JabRef
  • Screenshots added in PR description (for bigger UI changes)
  • Ensured that the git commit message is a good one
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Oct 20, 2018
Copy link
Member

@tobiasdiez tobiasdiez left a comment

Choose a reason for hiding this comment

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

I've trouble understanding the fix. Can you please add a bit more details about the changes and maybe add a test or two.

@@ -372,7 +372,7 @@ public String getName() {
FieldName.orFields(FieldName.YEAR, FieldName.DATE));
addAllOptional(FieldName.SUBTITLE, FieldName.TITLEADDON, FieldName.LANGUAGE, FieldName.HOWPUBLISHED,
FieldName.TYPE, FieldName.VERSION, FieldName.NOTE, FieldName.ORGANIZATION, FieldName.LOCATION,
FieldName.MONTH, FieldName.ADDENDUM, FieldName.PUBSTATE, FieldName.DOI, FieldName.EPRINT,
FieldName.DATE, FieldName.MONTH, FieldName.YEAR, FieldName.ADDENDUM, FieldName.PUBSTATE, FieldName.DOI, FieldName.EPRINT,
Copy link
Member

Choose a reason for hiding this comment

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

Why do you add Date and Year here? They fields are already in the required tab?

Copy link
Member Author

Choose a reason for hiding this comment

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

See biblatex manual, 2.1.1 misc entry type.
It's listed both in required and optional fields. Either it's an error in the spec or there is some reason why this is double?

Required felds: author/editor, title, year/date
Optional felds: subtitle, titleaddon, language, howpublished,
type, version, note, organization, location, date, month, year,
addendum, pubstate, doi, eprint, eprintclass, eprinttype, url,
urldate

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that looks to me like an error in manual.

Copy link
Member Author

Choose a reason for hiding this comment

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

It was confirmed as an error, created an issue at biblatex repo.

@@ -25,6 +25,6 @@ public OptionalFields2Tab(BibDatabaseContext databaseContext, SuggestionProvider

@Override
protected Collection<String> determineFieldsToShow(BibEntry entry, EntryType entryType) {
return entryType.getSecondaryOptionalNotDeprecatedFields();
return entryType.getSecondaryOptionalFields();
Copy link
Member

Choose a reason for hiding this comment

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

Doesn't this now also shows the deprecated fields in the second optional pane?

@Siedlerchr Siedlerchr closed this Nov 1, 2018
@tobiasdiez tobiasdiez deleted the fixMiscEntryEditor branch May 7, 2019 21:16
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
2 participants