-
Notifications
You must be signed in to change notification settings - Fork 302
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
Plagiarism check
: Fix display of escaped characters
#6123
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code changes look good 👍
…gfix/plagiarism-escpaed-matches
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code looks good 👍
d5accbb
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on TS1 using this programming exercise: https://artemis-test1.artemis.in.tum.de/course-management/40/programming-exercises/1917
-
Similarity distribution is displayed as expected: https://artemis-test1.artemis.in.tum.de/course-management/40/programming-exercises/1917/plagiarism
-
Code is displayed as expected: https://artemis-test1.artemis.in.tum.de/course-management/40/plagiarism-cases/84
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
…gfix/plagiarism-escpaed-matches
…gfix/plagiarism-escpaed-matches
70460c6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey I have a few questions regarding insertMatchTokens
:)
.../plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts
Outdated
Show resolved
Hide resolved
.../plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts
Outdated
Show resolved
Hide resolved
.../plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts
Outdated
Show resolved
Hide resolved
.../plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts
Show resolved
Hide resolved
.../plagiarism/plagiarism-split-view/text-submission-viewer/text-submission-viewer.component.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for addressing my comments. The code looks good now ✅
Checklist
General
Client
Motivation and Context
Plagiarism check
: Some characters are presented differently in the code comparison. #6120Description
Plagiarism checks
: Fix submission view for submissions with unescaped characters #5991 introduced escaping for the plagiarism view. However, this makes the resulting text longer, and the provided start and end indices of JPlag were no longer valid. In some cases, this leads to a highlighted text starting in between an escaped character as seen in the issue.JPlag 4.1.0 fixed this bug and now returns the correct order. Our workaround code is no longer needed. See Fix similarity distribution in JPlagResult jplag/JPlag#794
Steps for Testing
This is a bit tricky to reproduce since you need a JPlag match stating directly after an
>
character.Run a new plagiarism check and see that the chart displays the results correctly.
Test Coverage
unchanged