Skip to content

Fix IEEE preview does not display month #3983

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
May 2, 2018
Merged

Fix IEEE preview does not display month #3983

Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6738711
Fix IEEE preview does not display month (#3239)
DrSiroukane Apr 26, 2018
4d7525e
Fix IEEE preview does not display month (#3239)
DevSiroukane Apr 26, 2018
99c6dd5
Fix IEEE preview does not display month (#3239)
DevSiroukane Apr 26, 2018
cb5a8a3
Fix IEEE preview does not display month (#3239)
DevSiroukane Apr 26, 2018
0255d0f
Fix IEEE preview does not display month (#3239)
DevSiroukane Apr 28, 2018
2f022b2
Fix IEEE preview does not display month (#3239)
DevSiroukane Apr 28, 2018
82b05d1
Fix IEEE preview does not display month (#3239)
DevSiroukane Apr 29, 2018
f548163
Fix IEEE preview does not display month (#3239)
DevSiroukane Apr 30, 2018
eb2ad39
Fix IEEE preview does not display month (#3239)
DevSiroukane May 1, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions src/main/java/org/jabref/logic/citationstyle/CSLAdapter.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import org.jabref.logic.formatter.bibtexfields.RemoveNewlinesFormatter;
import org.jabref.logic.layout.format.HTMLChars;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.FieldName;

import de.undercouch.citeproc.CSL;
import de.undercouch.citeproc.ItemDataProvider;
Expand Down Expand Up @@ -84,13 +85,22 @@ private static class JabRefItemDataProvider implements ItemDataProvider {
* Converts the {@link BibEntry} into {@link CSLItemData}.
*/
private static CSLItemData bibEntryToCSLItemData(BibEntry bibEntry) {

// create a copy of bibEntry
Copy link
Member

Choose a reason for hiding this comment

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

This comment states the same as it is done in the code. So please "Remove Superfluous Comments" Java by Comparison.

You can also describe, why a clone is generated!

Copy link
Contributor Author

@DevSiroukane DevSiroukane Apr 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

Is this a good comment?

/**
 * Converts the {@link BibEntry} into {@link CSLItemData}.
 *
 * Clone bibEntry to make a save changes,
 * change month field from JabRefFormat <code>#mon#</> to ShortName <code>mon</code>
 * because CSL does not support JabRefFormat.
 */

Clone was generated because bibEntry is shared with many objects. So, to be sure that non of this objects may have issue after changing month field. I create a copy to change on it.

bibEntry = (BibEntry) bibEntry.clone();

String citeKey = bibEntry.getCiteKeyOptional().orElse("");
BibTeXEntry bibTeXEntry = new BibTeXEntry(new Key(bibEntry.getType()), new Key(citeKey));

// Not every field is already generated into latex free fields
HTMLChars latexToHtmlConverter = new HTMLChars();
RemoveNewlinesFormatter removeNewlinesFormatter = new RemoveNewlinesFormatter();
for (String key : bibEntry.getFieldMap().keySet()) {
if (FieldName.MONTH.equals(key)) {
// change month field value from "#mon#" to "mon"
Copy link
Member

@koppor koppor Apr 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I don't understand why this hack is necessary at this place.

In case a field value is surrounded by #, JabRef stores the value as follows:

month = mar

and not

month = #mar#

See http://help.jabref.org/en/EntryEditor#field-consistency-checking for an explanation. So, I think, either the wrong method is used to get the month or there is something more wrong on JabRef: All other exports should have difficulties.

Copy link
Member

Choose a reason for hiding this comment

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

This is actually necessary. The problem is that csl can not handle the JabRef special Syntax with the #. Therefore the value of the month field without the # is needed.

For export usually a latex formatter is applied which takes care of this

Copy link
Contributor Author

@DevSiroukane DevSiroukane Apr 28, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@koppor As CSL is not part of our code we cannot inject instructions that handle #mon# forma.
The issue on CSL plugin is still open Bibtex month like #mar# not parsed #41

bibEntry.getFieldMap().put(FieldName.MONTH, bibEntry.getMonth().get().getShortName());
}

bibEntry.getField(key)
.map(removeNewlinesFormatter::format)
.map(latexToHtmlConverter::format)
Copy link
Member

Choose a reason for hiding this comment

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

The code changes above work but are more complicated than they need to. I think an easier solution is to extract the lambda expression in this line (ifPresent(value -> ...)) to a new method setFieldValue(bibTeXEntry, key, value). In this new method, you can then simply check if (FieldName.MONTH.equals(key)) { and use Month.parse to get the correct month value.

Copy link
Contributor Author

@DevSiroukane DevSiroukane Apr 29, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

I did not notice that bibEntry and BibTeXEntry are different object. I think we do not need to clone bibEntry anymore.
Instead of create a new method setFieldValue(...) I thought of making if block direct on lamda expression. What do you think?

I execute this code and it work.

private static CSLItemData bibEntryToCSLItemData(BibEntry bibEntry) {
    String citeKey = bibEntry.getCiteKeyOptional().orElse("");
    BibTeXEntry bibTeXEntry = new BibTeXEntry(new Key(bibEntry.getType()), new Key(citeKey));

    // Not every field is already generated into latex free fields
    HTMLChars latexToHtmlConverter = new HTMLChars();
    RemoveNewlinesFormatter removeNewlinesFormatter = new RemoveNewlinesFormatter();
    for (String key : bibEntry.getFieldMap().keySet()) {
        bibEntry.getField(key)
                .map(removeNewlinesFormatter::format)
                .map(latexToHtmlConverter::format)
                .ifPresent(value -> {
                    if((FieldName.MONTH.equals(key)) && (value.matches("#[a-z]{3}#"))){
                        value = value.substring(1,4);
                    }
                    bibTeXEntry.addField(new Key(key), new DigitStringValue(value));
                });
    }
    return BIBTEX_CONVERTER.toItemData(bibTeXEntry);
}

About https://github.com/JabRef/jabref/blob/master/src/test/java/org/jabref/logic/citationstyle/CitationStyleTest.java I have no idea what I can do if you help me please. I am not used with test on java yet.

Copy link
Member

Choose a reason for hiding this comment

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

That is also an idea, but I would not make the regex. If you have the field month, I would really get the shortname from the BibEntry then, so value = key.getMonth().getShortName

Copy link
Contributor Author

Choose a reason for hiding this comment

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

for (String key : bibEntry.getFieldMap().keySet()) {
    bibEntry.getField(key)
            .map(removeNewlinesFormatter::format)
            .map(latexToHtmlConverter::format)
            .ifPresent(value -> {
                if(FieldName.MONTH.equals(key)){
                    value = bibEntry.getMonth().get().getShortName();
                }
                bibTeXEntry.addField(new Key(key), new DigitStringValue(value));
            });
}

Copy link
Member

Choose a reason for hiding this comment

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

Very good, we are almost there. Please replace the value assignment line by

value = bibEntry.getMonth().map(Month::getShortName).orElse(value);

Background: the get method for an Optional throws an null pointer exception if the optional is empty. Thus, one has to check isPresent before using get or, alternatively, use orElse instead.

Please push the change to your branch and then we can merge!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just post my last request

Expand Down