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.

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

Explicit conflict marker detection #629

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Explicit conflict marker detection #629

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
35b4185
Rename "getErrorMessage()" to "getWarningsAsString()"
koppor Sep 26, 2022
ac21220
Add toString() and refine test
koppor Sep 26, 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
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/cli/ArgumentProcessor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ private static Optional<ParserResult> importFile(String argument) {
importResult.ifPresent(result -> {
OutputPrinter printer = new SystemOutputPrinter();
if (result.hasWarnings()) {
printer.showMessage(result.getErrorMessage());
printer.showMessage(result.getWarningsAsString());
}
});
return importResult;
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/JabRefGUI.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ private void openDatabases() {
for (ParserResult pr : failed) {
String message = Localization.lang("Error opening file '%0'.",
pr.getPath().map(Path::toString).orElse("(File name unknown)")) + "\n" +
pr.getErrorMessage();
pr.getWarningsAsString();

mainFrame.getDialogService().showErrorDialogAndWait(Localization.lang("Error opening file"), message);
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/entryeditor/SourceTab.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -276,7 +276,7 @@ private void storeSource(BibEntry outOfFocusEntry, String text) {
if (parserResult.hasWarnings()) {
// put the warning into as exception text -> it will be displayed to the user

throw new IllegalStateException(parserResult.getErrorMessage());
throw new IllegalStateException(parserResult.getWarningsAsString());
}

NamedCompound compound = new NamedCompound(Localization.lang("source edit"));
Expand Down
4 changes: 2 additions & 2 deletions src/main/java/org/jabref/gui/externalfiles/ImportHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ protected List<ImportFilesResultItemViewModel> call() {
List<BibEntry> pdfEntriesInFile = pdfImporterResult.getDatabase().getEntries();

if (pdfImporterResult.hasWarnings()) {
addResultToList(file, false, Localization.lang("Error reading PDF content: %0", pdfImporterResult.getErrorMessage()));
addResultToList(file, false, Localization.lang("Error reading PDF content: %0", pdfImporterResult.getWarningsAsString()));
}

if (!pdfEntriesInFile.isEmpty()) {
Expand All @@ -127,7 +127,7 @@ protected List<ImportFilesResultItemViewModel> call() {
} else if (FileUtil.isBibFile(file)) {
var bibtexParserResult = contentImporter.importFromBibFile(file, fileUpdateMonitor);
if (bibtexParserResult.hasWarnings()) {
addResultToList(file, false, bibtexParserResult.getErrorMessage());
addResultToList(file, false, bibtexParserResult.getWarningsAsString());
}

entriesToAdd.addAll(bibtexParserResult.getDatabaseContext().getEntries());
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/gui/importer/actions/OpenDatabaseAction.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ private ParserResult loadDatabase(Path file) throws Exception {
Globals.getFileUpdateMonitor());
if (result.hasWarnings()) {
String content = Localization.lang("Please check your library file for wrong syntax.")
+ "\n\n" + result.getErrorMessage();
+ "\n\n" + result.getWarningsAsString();
DefaultTaskExecutor.runInJavaFXThread(() ->
dialogService.showWarningDialogAndWait(Localization.lang("Open library error"), content));
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/logic/importer/ImportFormatReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,7 @@ public UnknownFormatImport importUnknownFormat(Path filePath, FileUpdateMonitor
parserResult.setPath(filePath);
return new UnknownFormatImport(ImportFormatReader.BIBTEX_FORMAT, parserResult);
} else {
throw new ImportException(parserResult.getErrorMessage());
throw new ImportException(parserResult.getWarningsAsString());
}
} catch (IOException ignore) {
// Ignored
Expand Down
28 changes: 21 additions & 7 deletions src/main/java/org/jabref/logic/importer/ParserResult.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ public static ParserResult fromErrorMessage(String message) {
return parserResult;
}

private static String getErrorMessage(Exception exception) {
private static String getWarningsAsString(Exception exception) {
String errorMessage = exception.getLocalizedMessage();
if (exception.getCause() != null) {
errorMessage += " Caused by: " + exception.getCause().getLocalizedMessage();
Expand All @@ -61,7 +61,7 @@ private static String getErrorMessage(Exception exception) {
}

public static ParserResult fromError(Exception exception) {
return fromErrorMessage(getErrorMessage(exception));
return fromErrorMessage(getWarningsAsString(exception));
}

/**
Expand Down Expand Up @@ -111,7 +111,7 @@ public void addWarning(String s) {
}

public void addException(Exception exception) {
String errorMessage = getErrorMessage(exception);
String errorMessage = getWarningsAsString(exception);
addWarning(errorMessage);
}

Expand All @@ -123,6 +123,10 @@ public List<String> warnings() {
return new ArrayList<>(warnings);
}

public String getWarningsAsString() {
return String.join(" ", warnings());
}

public boolean isInvalid() {
return invalid;
}
Expand All @@ -131,10 +135,6 @@ public void setInvalid(boolean invalid) {
this.invalid = invalid;
}

public String getErrorMessage() {
return String.join(" ", warnings());
}

public BibDatabaseContext getDatabaseContext() {
return new BibDatabaseContext(database, metaData, file);
}
Expand All @@ -157,6 +157,20 @@ public boolean wasChangedOnMigration() {
return changedOnMigration;
}

@Override
public String toString() {
return "ParserResult{" +
"entryTypes=" + entryTypes +
", warnings=" + warnings +
", database=" + database +
", metaData=" + metaData +
", file=" + file +
", invalid=" + invalid +
", toOpenTab=" + toOpenTab +
", changedOnMigration=" + changedOnMigration +
'}';
}

public void setChangedOnMigration(boolean wasChangedOnMigration) {
this.changedOnMigration = wasChangedOnMigration;
}
Expand Down
2 changes: 1 addition & 1 deletion src/main/java/org/jabref/logic/importer/fetcher/MedlineFetcher.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ private List<BibEntry> fetchMedline(List<String> ids) throws FetcherException {
ParserResult result = new MedlineImporter().importDatabase(
new BufferedReader(new InputStreamReader(data.getInputStream(), StandardCharsets.UTF_8)));
if (result.hasWarnings()) {
LOGGER.warn(result.getErrorMessage());
LOGGER.warn(result.getWarningsAsString());
}
List<BibEntry> resultList = result.getDatabase().getEntries();
resultList.forEach(this::doPostCleanup);
Expand Down
12 changes: 12 additions & 0 deletions src/main/java/org/jabref/model/database/BibDatabase.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -646,4 +646,16 @@ public void setNewLineSeparator(String newLineSeparator) {
public String getNewLineSeparator() {
return newLineSeparator;
}

@Override
public String toString() {
return "BibDatabase{" +
"entries.size()=" + entries.size() +
", bibtexStrings.size()=" + bibtexStrings.size() +
", preamble='" + preamble + '\'' +
", epilog='" + epilog + '\'' +
", sharedDatabaseID='" + sharedDatabaseID + '\'' +
", newLineSeparator='" + newLineSeparator + '\'' +
'}';
}
}
23 changes: 23 additions & 0 deletions src/test/java/org/jabref/logic/importer/fileformat/BibtexParserTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1923,4 +1923,27 @@ void bibTeXConstantAprilIsParsedAsStringMonthAprilWhenReadingTheField() throws P

assertEquals(Optional.of("#apr#"), result.get().getField(StandardField.MONTH));
}

@Test
void conflictMarkersResultInProperParseError() throws Exception {
String input = """
@Article{first,
author = {Author},
<<<<<<< HEAD:test.bib
title = {Title},
=======
title = {My title},
>>>>>>> 77976da35a11db4580b80ae27e8d65caf5208086:test.bib
}

@Article{second,
author = {Valid Author},
}
""";
ParserResult parserResult = parser.parse(new StringReader(input));

ParserResult expected = ParserResult.fromErrorMessage("Found git conflict markers");

assertEquals(expected, parserResult);
Copy link
Member

Choose a reason for hiding this comment

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

I got the test passing by checking for git markers inside the read method after consuming a newline character.
I also called checkForGitMarkers at the beginning of the file to ensure it's called on the first line.

private int read() throws IOException {
        int character = pushbackReader.read();

        if (!isEOFCharacter(character)) {
            pureTextFromFile.offerLast((char) character);
        }
        if (character == '\n') {
            line++;
            checkForGitConflictMarker();
        }
        return character;
    }

This is the logic. It looks for a line that starts with the 'ours' marker, which is represented by the symbol <<<<<<<. Then it continues to skip lines until it reaches the 'theirs' marker >>>>>>>.

    private void checkForGitConflictMarker() throws IOException {
        skipSpace();
        int markerCount = 0;
        // Looking for the 'ours' marker
        char c;
        while ((c = (char) peek()) == '<' && !isEOFCharacter(c)) {
            read();
            markerCount++;
        }
        if (markerCount == 7) {
            parserResult.addWarning("Found git conflict markers at line %d".formatted(line));
            // Skip 'ours' marker <<<<<<<
            skipLine();
            // Keep skipping lines until we hit the beginning of 'theirs' marker >>>>>>>
            while (peek() != '>' && !isEOFCharacter(peek())) {
                skipLine();
            }
            // Skip 'theirs' marker if we haven't hit EOF already
            if (!isEOFCharacter(peek())) {
                skipLine();
            }
        }
    }

    private void skipLine() throws IOException {
        while (peek() != '\n' && !isEOFCharacter(peek())) {
            read();
        }
        skipOneNewline();
    }

I had to modify the test slightly to pass because the logic I used would continue parsing after the marker, resulting in a parser result with two entries when the expected parser result is zero. I changed it to check whether the warning list contains the git conflict warning.

       assertTrue(parserResult.warnings().contains("Found git conflict markers at line 3"));

Copy link
Member Author

Choose a reason for hiding this comment

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

Where is the commit? 🎉👀

Copy link
Member

Choose a reason for hiding this comment

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

I didn't make a commit. I got the inspiration of the solution while working on another PR, so I just made the changes on that PR's branch. However, you can use the code above inside BibTeXParser, and don't forget to set co-authored with me 😁.

}
}
2 changes: 1 addition & 1 deletion src/test/java/org/jabref/logic/importer/fileformat/ImporterTestEngine.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public static void testIsNotRecognizedFormat(Importer importer, String fileName)
public static void testImportEntries(Importer importer, String fileName, String fileType) throws IOException, ImportException {
ParserResult parserResult = importer.importDatabase(getPath(fileName));
if (parserResult.isInvalid()) {
throw new ImportException(parserResult.getErrorMessage());
throw new ImportException(parserResult.getWarningsAsString());
}
List<BibEntry> entries = parserResult.getDatabase()
.getEntries();
Expand Down