JabRef
Improve KeywordList parsing to support escaped characters and nested structures
Closes #12810
What I changed Implemented escape handling in KeywordList.parse(String, Character, Character) to support escaping the keyword separator using backslash (). This prevents incorrect splitting of keywords when delimiters appear within the keyword itself.
Refactored the parsing logic for clarity, including renaming loop variables for better readability and intent.
Added test cases in KeywordListTest to cover escape scenarios such as:
Escaped delimiter: "one\,two" → ["one,two"]
Escaped backslash: "one\\two" → ["one\two"]
Mixed escaped and unescaped delimiters.
Where the changes are KeywordList.java: Modified the parse method to include escape handling logic using a character-by-character loop.
KeywordListTest.java: Added JUnit test cases to ensure parsing behaves correctly with escaped delimiters and backslashes.
Why I made these changes Fixes issue #12810: Current parsing breaks when delimiters appear inside keywords (e.g., in MeSH terms). There was no way to escape the delimiter character, leading to incorrect keyword splitting.
Improves consistency: Provides a more robust, predictable behavior for keyword parsing, especially when users or importers use delimiters in keyword values.
Next Steps Awaiting review and feedback.
If the community prefers a different escape character or behavior, I’m happy to adjust the implementation.
After this is merged, similar logic could be extracted or reused where keyword parsing happens elsewhere (e.g., importers).
Mandatory checks
- [x] I own the copyright of the code submitted and I license it under the MIT license
- [/] Change in
CHANGELOG.mddescribed in a way that is understandable for the average user (if change is visible to the user) - [x] Tests created for changes (if applicable)
- [x] Manually tested changed features in running JabRef (always required)
- [/] Screenshots added in PR description (if change is visible to the user)
- [/] Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
- [x] Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.
can someone help me on this. is this a valid comment from Bot ? I have never used DisplayName annotation and I changed method name to be more comprehensive. Am I missing anything here ?
@krishnagjsForGit please change the PR title to contain text summarizing the fix.
@krishnagjsForGit please change the PR title to contain text summarizing the fix.
Done. is this fine @koppor ?
I'm sorry for not getting back to you sooner. I have two suggestions:
- ANTLR probably won't help in this use case, since the delimiters are dynamically specified
- I am concerned regarding the Unicode handling of the code, since Java char is finicky, and potentially you want to use codepoints instead of
.charAt - One way to address 2 and increase readability without necessarily using ANTLR is to dynamically construct a Regular Expression with a lookbehind and use only
.splitand.replace. Take a look at this: https://regex101.com/r/O1kLWF/1
I'm sorry for not getting back to you sooner. I have two suggestions:
- ANTLR probably won't help in this use case, since the delimiters are dynamically specified
- I am concerned regarding the Unicode handling of the code, since Java char is finicky, and potentially you want to use codepoints instead of
.charAt- One way to address 2 and increase readability without necessarily using ANTLR is to dynamically construct a Regular Expression with a lookbehind and use only
.splitand.replace. Take a look at this: https://regex101.com/r/O1kLWF/1
@koppor you want me to change the code accordingly ?
I think the current code is good.
Is a result of current JabRefs way of parsing. If it was done "correctly", the Bib parser would do the escape handling - and all other data structures would use tokens. But this is much much effort to change.
@krishnagjsForGit please add a CHANGELOG.md entry
Can you please check if I updated correctly ?
Seems like it was not a clean merge. @koppor may be able to help with some git trick Otherwise, you'll have to git reset and push a fresh commit.
Seems like it was not a clean merge. @koppor may be able to help with some git trick Otherwise, you'll have to git reset and push a fresh commit.
Just checking the output of GitHub here, I also suggest git reset origin/main after a git merge origin/main. - A force push afterwards is OK. Please use gitk --all& as a double check - and run this command before any action and leave the window open. You can press F5 to refresh.
Alternatively, one could work with git cherry-pick on a new branch - and just cherry-pick the "good" commits.
Something went amiss and PR got closed. Can I create a separate Clean PR and fix this ? @koppor
Something went amiss and PR got closed. Can I create a separate Clean PR and fix this ? @koppor
Yes, please.
I restored the state as of April, 16th - commit 081b9dc1d3. Mayb,e this helps.
If you do a git fetch origin and then git reset --hard origin/fix-for-issue-12810 you should be on the same page.
Attention: The command git reset --hard .... throws away all your work you did inbetween. Not sure if you did something...
![trag-bot[bot] avatar](https://avatars.githubusercontent.com/in/856808?v=4 "Published by a pub.dev verified publisher"){kind=small}
I saw no activtiy since 2 months, thus closing this and making it free for another contributor.
Your code currently does not meet JabRef's code guidelines. We use Checkstyle to identify issues. You can see which checks are failing by locating the box "Some checks were not successful" on the pull request page. To see the test output, locate "Tests / Checkstyle (pull_request)" and click on it.
In case of issues with the import order, double check that you activated Auto Import. You can trigger fixing imports by pressing Ctrl+Alt+O to trigger Optimize Imports.
Please carefully follow the setup guide for the codestyle. Afterwards, please run checkstyle locally and fix the issues, commit, and push.
