Year/date format suggestion

[grimes2]

Year/date format suggestion. The change adds a greyed-out suggestion for the recommended format of the field. Example: for the date field the format should be yyyy-mm-dd.

• Change in CHANGELOG.md described
• Tests created for changes
• Screenshots added (for bigger UI changes)
• Manually tested changed features in running JabRef
• Check documentation status (Issue created for outdated help page at help.jabref.org?)
• If you changed the localization: Did you run gradle localizationUpdate?

[koppor]

The idea is good. Shouldn't the ifs be else ifs?

Further date/time rework is discussed at JabRef#130.

Suggestion: you could elaborate a bit in the PR description. If one reads the notifications via mail, one cannot directly see the diff and get the idea.

[grimes2]

I'm not sure, but else if s were eventually skipped.

[grimes2]

Please set label "ready-for-review". Thanks.

[stefan-kolb]

Better suggest https://

[lenhard]

Tested locally and looks good! It is also interesting to see that we have not quite listed all BibLaTeX fields.

Very minor issue: Could you add a changelog entry? After all something visible has changed (New suggestions in certain fields in the entry editor). Then I would merge.

[koppor]

Could you also check the biblatex mode? editora, editorb, editorc, translator, ...?

[lenhard]

@koppor What exactly do you mean? Those fields (except URL) are only visible in the entry editor in biblatex mode anyway.

[grimes2]

Everything ok with editora, editorb... It only makes sense to define a field name constant for this fields, if you want to use the constant somewhere else in the code.

[lenhard]

@grimes2 @koppor Excellent hint! And also something additional to do in this PR!

Please have a look at net.sf.jabref.model.entry.BibLatexEntryTypes. Reuse your newly introduced field name constants in the definition of the entry types (where the string representations are still used directly). Also, you could introduce field name constants for every field name that is still used explicitly as string in the type definitions. This is, I think, what @koppor suggests. It would improve the types, since we would have more static checking.

[lenhard]

The replacement looks good!

Now, one more refactoring (and apologies for all the requests). After looking at the code a second time, I think @koppor is correct in that this should be if else blocks. After all, if field.equals(FieldName.YEAR) is true, field.equals(FieldName.DATE) can never be true. Furthermore, the code blocks should be reversed, e.g. FieldName.YEAR.equals(field). This has the same outcome, but we are very sure that FieldName.YEAR will never be null, whereas field might be just be for some buggy reason. The code would be a little more stable if equals is invoked on FieldName.YEAR (no NullPointerException). Finally, the whole prompt selection could be extracted into a new method private String getPrompt() or a similar name. This would make setupPanel a little less complex.

Thanks for your patience with our requests, we are just trying to get the best quality code :-)

[tobiasdiez]

why not just switch(field) instead of if + else?

After @lenhard's comments are implemented 👍 for merge.

[lenhard]

@tobiasdiez Because switches are ugly and if-elseifs are beautiful :-P

Let us leave that open for @grimes2

[Siedlerchr]

is the "k" after and a typo?

[Siedlerchr]

Code looks good to me!

@lenhard Switches are compiled to more efficient bytecode and instead of chained if/else ifs they are actually better readable:
http://www.artima.com/underthehood/flowP.html
http://stackoverflow.com/questions/6705955/why-switch-is-faster-than-if

[grimes2]

The general form of the ISSN code is NNNN-NNNC (Wikipedia). Should I add this form as a suggestion to the ISSN field?

[matthiasgeiger]

I think a Suggestion for ISSN is not necessary - there is only one existing Format. And if you search for the ISSN of Journal it should be found in this format.

[lenhard]

@Siedlerchr: Switches are the source of many infamous switch-fall-through bugs (Apple's certificate validation system for example, if I remember it correctly), which is why I try to avoid them. To me, a potential increase in maintainability beats a potential improvement in performance any day. But anyway, there is not really a point in arguing about this.

To me the current solution is fine and I am merging it now. @grimes2 Thank you very much for your contribution!

[koppor]

Sorry for the late reply. I mean that at "Optional fields 2", all fields containing persons should also contain such a help text, shouldn't they?

At editor, it works (even though, DOI could also have a help text to prevent users keying in the https://dx.doi.org/ prefix)

[grimes2]

@bartsch-dev refactors the suggestion switch in EntryEditorTab (#2199). He can easily add further cases. In DOI field is the DOI-Resolver https://dx.doi.org/ not allowed? The Standard DOI is in the form10.ORGANISATION/ID