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

Explicit conflict marker detection #629

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
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
Prev Previous commit
Add toString() and refine test
Co-authored-by: Christoph <[email protected]>
Co-authored-by: Carl Christian Snethlage <[email protected]>
Co-authored-by: Houssem Nasri <[email protected]>
Co-authored-by: Benedikt Tutzer <[email protected]>
  • Loading branch information
5 people committed Sep 26, 2022
commit ac21220f213be501781053a4eb49c794b2603472
14 changes: 14 additions & 0 deletions src/main/java/org/jabref/logic/importer/ParserResult.java
Original file line number Diff line number Diff line change
@@ -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;
}
12 changes: 12 additions & 0 deletions src/main/java/org/jabref/model/database/BibDatabase.java
Original file line number Diff line number Diff line change
@@ -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 + '\'' +
'}';
}
}
Original file line number Diff line number Diff line change
@@ -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 😁.

}
}