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

Fix Abbreviation for Escaped Ampersand #9288

Merged
merged 26 commits into from
Nov 2, 2022
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
ed40cc2
Added Naive Ampersand Formatter, see last point in #8948 to-do
AkshatJain9 Oct 14, 2022
3dc87ae
Rudimentary attempt at allow \& in the journal abbreviation. However,…
ANUu7312578 Oct 15, 2022
e56af9e
Remove unnecessary code in BibWriter.java and unnecessary replacement…
ANUu7312578 Oct 20, 2022
ff15343
Merge branch 'main' of https://github.com/JabRef/jabref
ANUu7312578 Oct 20, 2022
bc31f93
Merge branch 'main' of https://github.com/AkshatJain9/jabref into iss…
ANUu7312578 Oct 20, 2022
4479e19
Merge branch 'JabRef:main' into main
AkshatJain9 Oct 22, 2022
03a996b
Merge branch 'JabRef:main' into issue-8948
AkshatJain9 Oct 22, 2022
1beef73
Merge branch 'main' of https://github.com/AkshatJain9/jabref into iss…
ANUu7312578 Oct 22, 2022
26b6527
Remove linebreak for checkstyle
ANUu7312578 Oct 22, 2022
97fdb55
Updated CHANGELOG.md to reflect the changes made
ANUu7312578 Oct 22, 2022
d34b5cd
Create tests for escaped ampersand in abbreviation
ANUu7312578 Oct 23, 2022
d8d0850
Merge branch 'main' of https://github.com/JabRef/jabref
ANUu7312578 Oct 25, 2022
64cc386
Merge branch 'main' of https://github.com/AkshatJain9/jabref into iss…
ANUu7312578 Oct 25, 2022
5426806
Remove unnecessary comments
ANUu7312578 Oct 25, 2022
0ed991b
Convert to using getResolvedFieldOrAliasLatexFree instead of replace()
ANUu7312578 Oct 26, 2022
d8a69c8
Added a javadoc to explain the function created
ANUu7312578 Oct 26, 2022
0efdc5a
Merge branch 'main' of https://github.com/JabRef/jabref
ANUu7312578 Oct 26, 2022
18d7d8b
Merge branch 'main' of https://github.com/AkshatJain9/jabref into iss…
ANUu7312578 Oct 26, 2022
e18c792
Added 3 tests to BibEntryWriterTest which test abbreviating journal e…
ANUu7312578 Oct 28, 2022
f4658bc
Merge branch 'main' of https://github.com/JabRef/jabref
ANUu7312578 Oct 28, 2022
943fa5b
Merge branch 'main' of https://github.com/AkshatJain9/jabref into iss…
ANUu7312578 Oct 28, 2022
7815ba0
Move journal abbreviation tests from BibEntryWriterTest to JournalAbb…
ANUu7312578 Oct 28, 2022
fc0907b
CHANGELOG.md changed to remove unnecessary information. restrictUsage…
ANUu7312578 Oct 29, 2022
0657b7e
Merge branch 'main' of https://github.com/JabRef/jabref
ANUu7312578 Oct 29, 2022
046498b
Merge branch 'main' of https://github.com/AkshatJain9/jabref into iss…
ANUu7312578 Oct 29, 2022
098bdac
Merge branch 'main' into issue-8948
Siedlerchr Nov 1, 2022
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,7 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We fixed an issue where hitting enter on the search field within the preferences dialog closed the dialog. [koppor#630](https://github.com/koppor/jabref/issues/630)
- We fixed a typo within a connection error message. [koppor#625](https://github.com/koppor/jabref/issues/625)
- We fixed an issue where the 'close dialog' key binding was not closing the Preferences dialog. [#8888](https://github.com/jabref/jabref/issues/8888)
- We fixed an issue where journal abbreviations would not abbreviate journal titles with escaped ampersands (\\&) which are written in accordance with Latex format. [#8948](https://github.com/JabRef/jabref/issues/8948)
Copy link
Member

Choose a reason for hiding this comment

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

I would remove the last part, that information should be made available in the internet elsewhere 😃

Suggested change
- We fixed an issue where journal abbreviations would not abbreviate journal titles with escaped ampersands (\\&) which are written in accordance with Latex format. [#8948](https://github.com/JabRef/jabref/issues/8948)
- We fixed an issue where journal abbreviations would not abbreviate journal titles with escaped ampersands (\\&). [#8948](https://github.com/JabRef/jabref/issues/8948)


### Removed

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@
import java.util.Set;
import java.util.stream.Collectors;

import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.field.StandardField;

import org.h2.mvstore.MVMap;
import org.h2.mvstore.MVStore;

Expand Down Expand Up @@ -48,7 +51,7 @@ private static boolean isMatchedAbbreviated(String name, Abbreviation abbreviati
* Letters) or its abbreviated form (e.g. Phys. Rev. Lett.).
*/
public boolean isKnownName(String journalName) {
String journal = journalName.trim();
String journal = getLatexFreeJournal(journalName.trim());

boolean isKnown = customAbbreviations.stream().anyMatch(abbreviation -> isMatched(journal, abbreviation));
if (isKnown) {
Expand All @@ -58,6 +61,22 @@ public boolean isKnownName(String journalName) {
return fullToAbbreviation.containsKey(journal) || abbreviationToFull.containsKey(journal);
}

/**
* Returns the LaTeX free version of a journal (e.g., IEEE Design \& Test would be returned as IEEE Design & Test)
* i.e., the journal name should not contain any unnecessary LaTeX syntax such as escaped ampersands
*
* @param journal The journal name
* @return The LaTeX free version of the journal name
*/
private String getLatexFreeJournal(String journal) {
BibEntry entry = new BibEntry();
entry.setField(StandardField.JOURNAL, journal);
if (entry.getResolvedFieldOrAliasLatexFree(StandardField.JOURNAL, null).isPresent()) {
journal = entry.getResolvedFieldOrAliasLatexFree(StandardField.JOURNAL, null).get();
}
return journal;
}

Copy link
Member

Choose a reason for hiding this comment

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

This was not the intention - sorry for that!

Your old code with the replacement was better!

I thought, you were reading the BibTeX field content at some place. Here, you seem to read the journal name from somewhere.

/**
* Returns true if the given journal name is in its abbreviated form (e.g. Phys. Rev. Lett.). The test is strict,
* i.e. journals whose abbreviation is the same as the full name are not considered
Expand All @@ -75,7 +94,7 @@ public boolean isAbbreviatedName(String journalName) {
* @param input The journal name (either abbreviated or full name).
*/
public Optional<Abbreviation> get(String input) {
String journal = input.trim();
String journal = getLatexFreeJournal(input.trim());

Optional<Abbreviation> customAbbreviation = customAbbreviations.stream()
.filter(abbreviation -> isMatched(journal, abbreviation))
Expand Down
4 changes: 2 additions & 2 deletions src/test/java/org/jabref/logic/bibtex/BibEntryWriterTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -856,7 +856,7 @@ static Stream<Arguments> testGetFormattedFieldNameData() {
@MethodSource("testGetFormattedFieldNameData")
void testGetFormattedFieldName(String expected, String fieldName, int indent) {
Field field = FieldFactory.parseField(fieldName);
assertEquals(expected, bibEntryWriter.getFormattedFieldName(field, indent));
assertEquals(expected, BibEntryWriter.getFormattedFieldName(field, indent));
}

static Stream<Arguments> testGetLengthOfLongestFieldNameData() {
Expand All @@ -871,6 +871,6 @@ static Stream<Arguments> testGetLengthOfLongestFieldNameData() {
@ParameterizedTest
@MethodSource("testGetLengthOfLongestFieldNameData")
void testGetLengthOfLongestFieldName(int expected, BibEntry entry) {
assertEquals(expected, bibEntryWriter.getLengthOfLongestFieldName(entry));
assertEquals(expected, BibEntryWriter.getLengthOfLongestFieldName(entry));
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,27 @@
package org.jabref.logic.journals;

import java.io.IOException;
import java.io.StringWriter;
import java.util.List;

import javax.swing.undo.CompoundEdit;

import org.jabref.gui.journals.AbbreviationType;
import org.jabref.gui.journals.UndoableAbbreviator;
import org.jabref.gui.journals.UndoableUnabbreviator;
import org.jabref.logic.bibtex.BibEntryWriter;
import org.jabref.logic.bibtex.FieldContentFormatterPreferences;
import org.jabref.logic.bibtex.FieldWriter;
import org.jabref.logic.bibtex.FieldWriterPreferences;
import org.jabref.logic.exporter.BibWriter;
import org.jabref.logic.util.OS;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.database.BibDatabaseMode;
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.BibEntryTypesManager;
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.entry.types.StandardEntryType;

import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;

Expand Down Expand Up @@ -134,4 +156,97 @@ void getFromFullName() {
void getFromAbbreviatedName() {
assertEquals(new Abbreviation("American Journal of Public Health", "Am. J. Public Health"), repository.get("Am. J. Public Health").get());
}

@Test
void testAbbreviationsWithEscapedAmpersand() {
assertEquals(new Abbreviation("ACS Applied Materials & Interfaces", "ACS Appl. Mater. Interfaces"), repository.get("ACS Applied Materials & Interfaces").get());
assertEquals(new Abbreviation("ACS Applied Materials & Interfaces", "ACS Appl. Mater. Interfaces"), repository.get("ACS Applied Materials \\& Interfaces").get());
assertEquals(new Abbreviation("Antioxidants & Redox Signaling", "Antioxid. Redox Signaling"), repository.get("Antioxidants & Redox Signaling").get());
assertEquals(new Abbreviation("Antioxidants & Redox Signaling", "Antioxid. Redox Signaling"), repository.get("Antioxidants \\& Redox Signaling").get());

repository.addCustomAbbreviation(new Abbreviation("Long & Name", "L. N.", "LN"));
assertEquals(new Abbreviation("Long & Name", "L. N.", "LN"), repository.get("Long & Name").get());
assertEquals(new Abbreviation("Long & Name", "L. N.", "LN"), repository.get("Long \\& Name").get());
Comment on lines +152 to +159
Copy link
Member

Choose a reason for hiding this comment

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

Normally, we expect one assertation per test case. Thus, this method should be split into several ones. @ParamterizedTest is the the thign to use.

}

@Test
void testJournalAbbreviationWithEscapedAmpersand() throws IOException {
FieldWriterPreferences fieldWriterPreferences = new FieldWriterPreferences(true, List.of(StandardField.MONTH), new FieldContentFormatterPreferences());
BibEntryWriter bibEntryWriter = new BibEntryWriter(new FieldWriter(fieldWriterPreferences), new BibEntryTypesManager());
StringWriter stringWriter = new StringWriter();
BibWriter bibWriter = new BibWriter(stringWriter, OS.NEWLINE);

BibDatabase bibDatabase = new BibDatabase();
JournalAbbreviationRepository journalAbbreviationRepository = JournalAbbreviationLoader.loadBuiltInRepository();
UndoableAbbreviator undoableAbbreviator = new UndoableAbbreviator(journalAbbreviationRepository, AbbreviationType.DEFAULT);

BibEntry entryWithEscapedAmpersandInJournal = new BibEntry(StandardEntryType.Article);
entryWithEscapedAmpersandInJournal.setField(StandardField.JOURNAL, "ACS Applied Materials \\& Interfaces");
Comment on lines +168 to +169
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
BibEntry entryWithEscapedAmpersandInJournal = new BibEntry(StandardEntryType.Article);
entryWithEscapedAmpersandInJournal.setField(StandardField.JOURNAL, "ACS Applied Materials \\& Interfaces");
BibEntry entryWithEscapedAmpersandInJournal = new BibEntry(StandardEntryType.Article)
.withField(StandardField.JOURNAL, "ACS Applied Materials \\& Interfaces");


// @formatter:off
String expectedAbbreviatedJournalEntry = """
@Article{,
journal = {ACS Appl. Mater. Interfaces},
}
""".replaceAll("\n", OS.NEWLINE);
// @formatter:on

undoableAbbreviator.abbreviate(bibDatabase, entryWithEscapedAmpersandInJournal, StandardField.JOURNAL, new CompoundEdit());
bibEntryWriter.write(entryWithEscapedAmpersandInJournal, bibWriter, BibDatabaseMode.BIBLATEX);
assertEquals(expectedAbbreviatedJournalEntry, stringWriter.toString());
Copy link
Member

Choose a reason for hiding this comment

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

Comparison on a String bases should only be used for serialization tests.

In your case, do

Suggested change
// @formatter:off
String expectedAbbreviatedJournalEntry = """
@Article{,
journal = {ACS Appl. Mater. Interfaces},
}
""".replaceAll("\n", OS.NEWLINE);
// @formatter:on
undoableAbbreviator.abbreviate(bibDatabase, entryWithEscapedAmpersandInJournal, StandardField.JOURNAL, new CompoundEdit());
bibEntryWriter.write(entryWithEscapedAmpersandInJournal, bibWriter, BibDatabaseMode.BIBLATEX);
assertEquals(expectedAbbreviatedJournalEntry, stringWriter.toString());
undoableAbbreviator.abbreviate(bibDatabase, entryWithEscapedAmpersandInJournal, StandardField.JOURNAL, new CompoundEdit());
BibEntry expectedAbbreviatedJournalEntry = new BibEntry(StandardEntryType.Article)
.withField(StandardField.JOURNAL, "ACS Appl. Mater. Interfaces");
assertEquals(expectedAbbreviatedJournalEntry , entryWithEscapedAmpersandInJournal);

}

@Test
void testJournalUnabbreviate() throws IOException {
FieldWriterPreferences fieldWriterPreferences = new FieldWriterPreferences(true, List.of(StandardField.MONTH), new FieldContentFormatterPreferences());
BibEntryWriter bibEntryWriter = new BibEntryWriter(new FieldWriter(fieldWriterPreferences), new BibEntryTypesManager());
StringWriter stringWriter = new StringWriter();
BibWriter bibWriter = new BibWriter(stringWriter, OS.NEWLINE);

BibDatabase bibDatabase = new BibDatabase();
JournalAbbreviationRepository journalAbbreviationRepository = JournalAbbreviationLoader.loadBuiltInRepository();
UndoableUnabbreviator undoableUnabbreviator = new UndoableUnabbreviator(journalAbbreviationRepository);

BibEntry abbreviatedJournalEntry = new BibEntry(StandardEntryType.Article);
abbreviatedJournalEntry.setField(StandardField.JOURNAL, "ACS Appl. Mater. Interfaces");

// @formatter:off
String expectedUnabbreviatedJournalEntry = """
@Article{,
journal = {ACS Applied Materials & Interfaces},
}
""".replaceAll("\n", OS.NEWLINE);
// @formatter:on

undoableUnabbreviator.unabbreviate(bibDatabase, abbreviatedJournalEntry, StandardField.JOURNAL, new CompoundEdit());
bibEntryWriter.write(abbreviatedJournalEntry, bibWriter, BibDatabaseMode.BIBLATEX);
assertEquals(expectedUnabbreviatedJournalEntry, stringWriter.toString());
}

@Test
void testJournalAbbreviateWithoutEscapedAmpersand() throws IOException {
FieldWriterPreferences fieldWriterPreferences = new FieldWriterPreferences(true, List.of(StandardField.MONTH), new FieldContentFormatterPreferences());
BibEntryWriter bibEntryWriter = new BibEntryWriter(new FieldWriter(fieldWriterPreferences), new BibEntryTypesManager());
StringWriter stringWriter = new StringWriter();
BibWriter bibWriter = new BibWriter(stringWriter, OS.NEWLINE);

BibDatabase bibDatabase = new BibDatabase();
JournalAbbreviationRepository journalAbbreviationRepository = JournalAbbreviationLoader.loadBuiltInRepository();
UndoableAbbreviator undoableAbbreviator = new UndoableAbbreviator(journalAbbreviationRepository, AbbreviationType.DEFAULT);

BibEntry entryWithoutEscapedAmpersandInJournal = new BibEntry(StandardEntryType.Article);
entryWithoutEscapedAmpersandInJournal.setField(StandardField.JOURNAL, "ACS Applied Materials & Interfaces");

// @formatter:off
String expectedAbbreviatedJournalEntry = """
@Article{,
journal = {ACS Appl. Mater. Interfaces},
}
""".replaceAll("\n", OS.NEWLINE);
// @formatter:on

undoableAbbreviator.abbreviate(bibDatabase, entryWithoutEscapedAmpersandInJournal, StandardField.JOURNAL, new CompoundEdit());
bibEntryWriter.write(entryWithoutEscapedAmpersandInJournal, bibWriter, BibDatabaseMode.BIBLATEX);
assertEquals(expectedAbbreviatedJournalEntry, stringWriter.toString());
}
}