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 all 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 @@ -80,6 +80,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 (\\&). [#8948](https://github.com/JabRef/jabref/issues/8948)
- We fixed an issue where font size preferences did not apply correctly to preference dialog window and the menu bar. [#8386](https://github.com/JabRef/jabref/issues/8386) and [#9279](https://github.com/JabRef/jabref/issues/9279)
- We fixed an issue when using an unsafe character in the citation key, the auto-linking feature fails to link files. [#9267](https://github.com/JabRef/jabref/issues/9267)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.regex.Matcher;
import java.util.stream.Collectors;

import org.h2.mvstore.MVMap;
Expand Down Expand Up @@ -48,7 +49,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 = journalName.trim().replaceAll(Matcher.quoteReplacement("\\&"), "&");
Copy link
Member

Choose a reason for hiding this comment

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

I think, there is the possibility to pre-compile the Regex. Was it with the Pattern class?


boolean isKnown = customAbbreviations.stream().anyMatch(abbreviation -> isMatched(journal, abbreviation));
if (isKnown) {
Expand All @@ -75,7 +76,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 = input.trim().replaceAll(Matcher.quoteReplacement("\\&"), "&");

Optional<Abbreviation> customAbbreviation = customAbbreviations.stream()
.filter(abbreviation -> isMatched(journal, abbreviation))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ public static void restrictUsagesInModel(JavaClasses classes) {
@ArchTest
public static void restrictUsagesInLogic(JavaClasses classes) {
noClasses().that().resideInAPackage(PACKAGE_ORG_JABREF_LOGIC)
.and().areNotAnnotatedWith(AllowedToUseSwing.class)
.should().dependOnClassesThat().resideInAPackage(PACKAGE_JAVAX_SWING)
.orShould().dependOnClassesThat().haveFullyQualifiedName(CLASS_ORG_JABREF_GLOBALS)
.check(classes);
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,12 +1,24 @@
package org.jabref.logic.journals;

import javax.swing.undo.CompoundEdit;

import org.jabref.architecture.AllowedToUseSwing;
import org.jabref.gui.journals.AbbreviationType;
import org.jabref.gui.journals.UndoableAbbreviator;
import org.jabref.gui.journals.UndoableUnabbreviator;
import org.jabref.model.database.BibDatabase;
import org.jabref.model.entry.BibEntry;
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;

import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;

@AllowedToUseSwing("UndoableUnabbreviator and UndoableAbbreviator requires Swing Compound Edit in order test the abbreviation and unabreviation of journal titles")
class JournalAbbreviationRepositoryTest {

private JournalAbbreviationRepository repository;
Expand Down Expand Up @@ -134,4 +146,61 @@ 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() {
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");


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() {
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");

undoableUnabbreviator.unabbreviate(bibDatabase, abbreviatedJournalEntry, StandardField.JOURNAL, new CompoundEdit());
BibEntry expectedAbbreviatedJournalEntry = new BibEntry(StandardEntryType.Article)
.withField(StandardField.JOURNAL, "ACS Applied Materials & Interfaces");
assertEquals(expectedAbbreviatedJournalEntry, abbreviatedJournalEntry);
}

@Test
void testJournalAbbreviateWithoutEscapedAmpersand() {
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");

undoableAbbreviator.abbreviate(bibDatabase, entryWithoutEscapedAmpersandInJournal, StandardField.JOURNAL, new CompoundEdit());
BibEntry expectedAbbreviatedJournalEntry = new BibEntry(StandardEntryType.Article)
.withField(StandardField.JOURNAL, "ACS Appl. Mater. Interfaces");
assertEquals(expectedAbbreviatedJournalEntry, entryWithoutEscapedAmpersandInJournal);
}
}