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: Ignore "Math superscript (^) and subscript (_) characters are not allowed in text mode" error for citation key (#11948) #12391

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Note that this project **does not** adhere to [Semantic Versioning](https://semv
- We fixed an issue where a bib file with UFF-8 charset was wrongly loaded with a different charset [forum#5369](https://discourse.jabref.org/t/jabref-5-15-opens-bib-files-with-shift-jis-encoding-instead-of-utf-8/5369/)
- We fixed an issue where new entries were inserted in the middle of the table instead of at the end. [#12371](https://github.com/JabRef/jabref/pull/12371)
- We fixed an issue where removing the sort from the table did not restore the original order. [#12371](https://github.com/JabRef/jabref/pull/12371)
- We fixed an issue where citation keys containing superscript (`^`) and subscript (`_`) characters in text mode were incorrectly flagged by the integrity checker. [#12391](https://github.com/JabRef/jabref/pull/12391)
- We fixed an issue where JabRef icon merges with dark background [#7771](https://github.com/JabRef/jabref/issues/7771)
- We fixed an issue where an entry's group was no longer highlighted on selection [#12413](https://github.com/JabRef/jabref/issues/12413)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.FieldFactory;
import org.jabref.model.entry.field.InternalField;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand Down Expand Up @@ -49,8 +50,6 @@ public class LatexIntegrityChecker implements EntryChecker {
snugglePackage.addComplexCommand("textless", false, 0, TEXT_MODE_ONLY, null, null, null);
snugglePackage.addComplexCommand("textbackslash", false, 0, TEXT_MODE_ONLY, null, null, null);
snugglePackage.addComplexCommand("textbar", false, 0, TEXT_MODE_ONLY, null, null, null);
// ENGINE.getPackages().get(0).addComplexCommandOneArg()
// engine.getPackages().get(0).addComplexCommandOneArg("text", false, ALL_MODES,LR, StyleDeclarationInterpretation.NORMALSIZE, null, TextFlowContext.ALLOW_INLINE);

SESSION = ENGINE.createSession();
SESSION.getConfiguration().setFailingFast(true);
Expand All @@ -66,6 +65,9 @@ public List<IntegrityMessage> check(BibEntry entry) {
.flatMap(LatexIntegrityChecker::getUnescapedAmpersandsWithCount)
// Exclude all DOM building errors as this functionality is not used.
.filter(pair -> !pair.getValue().getErrorCode().getErrorGroup().equals(CoreErrorGroup.TDE))
// Exclude TTEM03 error for citation key field
.filter(pair -> !(pair.getValue().getErrorCode().equals(CoreErrorCode.TTEM03) && pair.getKey().equals(InternalField.KEY_FIELD)))
// Exclude errors present in EXCLUDED_ERRORS set
Copy link
Member

Choose a reason for hiding this comment

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

Why this comment? This is really what is in the next line - normally one adds the why on top - or summarizes the next line. Please just remove line 70

Copy link
Author

Choose a reason for hiding this comment

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

Got it! Thanks for pointing that out! I’ll remove line 70.

.filter(pair -> !EXCLUDED_ERRORS.contains(pair.getValue().getErrorCode()))
.map(pair ->
new IntegrityMessage(errorMessageFormatHelper(pair.getValue().getErrorCode(), pair.getValue().getArguments()), entry, pair.getKey()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

import org.jabref.model.entry.BibEntry;
import org.jabref.model.entry.field.Field;
import org.jabref.model.entry.field.InternalField;
import org.jabref.model.entry.field.StandardField;
import org.jabref.model.entry.field.UserSpecificCommentField;

Expand Down Expand Up @@ -300,10 +301,33 @@ private static Stream<Arguments> provideUnacceptedInputs() {
Arguments.of(LatexIntegrityChecker.errorMessageFormatHelper(CoreErrorCode.TFEM03), StandardField.TITLE, "$\\right)$"),

// TFEM04: \left had no following \right
Arguments.of(LatexIntegrityChecker.errorMessageFormatHelper(CoreErrorCode.TFEM04), StandardField.TITLE, "$\\left($")
Arguments.of(LatexIntegrityChecker.errorMessageFormatHelper(CoreErrorCode.TFEM04), StandardField.TITLE, "$\\left($"),

// TFETB0: \hline must be the only token in table row
// Skipped

Arguments.of(LatexIntegrityChecker.errorMessageFormatHelper(CoreErrorCode.TTEM03), StandardField.TITLE, "Title with ^ and _ characters"),
// Ensure invalid LaTeX syntax with special characters in other fields is detected
Arguments.of(LatexIntegrityChecker.errorMessageFormatHelper(CoreErrorCode.TTEM03), StandardField.AUTHOR, "Author_Name"),
Arguments.of(LatexIntegrityChecker.errorMessageFormatHelper(CoreErrorCode.TTEM03), StandardField.JOURNAL, "Journal_Name"),
Arguments.of(LatexIntegrityChecker.errorMessageFormatHelper(CoreErrorCode.TTEM03), StandardField.BOOKTITLE, "Book^Title"),
Arguments.of(LatexIntegrityChecker.errorMessageFormatHelper(CoreErrorCode.TTEM03), StandardField.NOTE, "Note_with_subscript_and_superscript^_")
Comment on lines +309 to +314
Copy link
Member

Choose a reason for hiding this comment

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

OK, but you did not test for org.jabref.model.entry.field.InternalField#KEY_FIELD, where you changed the behavior.

Copy link
Author

@Satyamkumarnavneet Satyamkumarnavneet Jan 16, 2025

Choose a reason for hiding this comment

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

Thanks for pointing that out! I've added a new test case for InternalField.KEY_FIELD, but it's currently failing.

Copy link
Member

Choose a reason for hiding this comment

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

I assume, it is not failing now?

);
}

@ParameterizedTest
@MethodSource("provideCitationKeyInputs")
void citationKeyField(String expectedMessage, Field field, String value) {
entry.setField(field, value);
// Ensure TTEM03 error is not raised for citation key field
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 has to move above Arguments.of("", InternalField.KEY_FIELD, "Corti_2009"), to explain the _ in the inputs. -- In this place, it does not make sense.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, it's not failing now. It's working for all pre-existing tests as well as the newly added ones.

Screenshot 2025-02-03 at 7 34 58 PM

assertEquals(List.of(), checker.check(entry));
}

koppor marked this conversation as resolved.
Show resolved Hide resolved
private static Stream<Arguments> provideCitationKeyInputs() {
return Stream.of(
Arguments.of("", InternalField.KEY_FIELD, "Corti_2009"),
Arguments.of("", InternalField.KEY_FIELD, "Key_With^Superscript"),
Arguments.of("", InternalField.KEY_FIELD, "Key_With_Subscript")
);
}
}
Loading