JabRef
Duplicate Manager - Show Diff - Highlight characters --> highlights wrong characters
JabRef 5.8 commit 0b580790d7a323ccdb445b8a8a6e8dd4d5917e69
How to reproduce:
Have two entries with groups:
A) groups = {test1, test2, test3}, B) groups = {test1, test4, test2, test3},
Reality:
Expected:
This can be done by changing the default diff separator to the groups separator , when highlighting groups differences. This can be useful for highlighting keywords and files differences too.
https://github.com/JabRef/jabref/blob/bd77d7336fde1c4618f2e67dcf2ef22ceb1cda77/src/main/java/org/jabref/gui/mergeentries/newmergedialog/diffhighlighter/DiffHighlighter.java#L38-L40
Thank you :-)
As a general advice for newcomers: check out https://github.com/JabRef/jabref/blob/main/CONTRIBUTING.md for a start. Also, https://devdocs.jabref.org/getting-into-the-code/guidelines-for-setting-up-a-local-workspace is worth having a look at. Feel free to ask if you have any questions here on GitHub or also at JabRef's Gitter chat.
Try to open a (draft) pull request early on, so that people can see you are working on the issue and so that they can see the direction the pull request is heading towards. This way, you will likely receive valuable feedback.
@HoussemNasri Did you mean this preference?
I think the diff separator should probably always be similar to what the user has set in this preference
No, they are totally different. The diff highlighter separator can be either a space or an empty string. It's used for splitting the text before comparison. For example for diffing abcd w and ebdd sa, we can split the texts by space and compare them: abcd vs ebdd and w vs sa, or we can compare them character by character (split by an empty string): a vs e, b vs b, etc.
This can be done by changing the default diff separator to the groups separator
,when highlighting groups differences. This can be useful for highlighting keywords and files differences too.https://github.com/JabRef/jabref/blob/bd77d7336fde1c4618f2e67dcf2ef22ceb1cda77/src/main/java/org/jabref/gui/mergeentries/newmergedialog/diffhighlighter/DiffHighlighter.java#L38-L40
Hi @ThiloteE,
-
I followed the above method and created a pull request. Could you please review my codes? I made the following changes：
-
When calling showDiff method, I first check if the field is "Groups", if the field is Groups, I use "," as a seperator. I didn't change the seperator of other field. And the result shows as follows:
-
However, this method has its shortcoming, it only works when the user chooses to use "," to seperate different groups. Otherwise, the results will be like the following:
-
To solve this bug, I also choose to check if the leftValue or rightValue contains(","), if neither of the value contains(","), I choose to still use "" as the seperator. The results is like the following, kind of solving the issue.
-
@HoussemNasri said the seperator can also be used in the field "Keywords", however I think it‘s unnecessary. As the issue only occurs when each word has the same part (i.e "test"), the keywords field unlikely to have many words containing the same part.
-
However, I know this method doesn't solve the whold issue. I also tried if I could seperate the field value with "," first and then separate with ""; or use other methods in the com.github.difflib.DiffUtils (the library we use). Both of these two methods seem to need to change a lot of places and I failed. If you have any other suggestions, plz let me know.
Hi @ThiloteE and @HoussemNasri , I found myself failed a lot of styleCheck, should I create another pull request or just reset git history and push again on the same pull request? thank you 😊
I think you can simply create another commit to your pull request by pushing towards the same branch