Fix #3359: Automatically remove colon and apostrophe from key pattern#3506
Conversation
There was a problem hiding this comment.
Looks like a nice refactoring overall, which makes the key generator much more pleasant to use in code, I like that. I have nothing to criticize code-wise. But you still seem to have some checkstyle issues, if I am not mistaken.
I have tested this locally and key generation works and leaves out the characters we want to avoid. I am a still a little doubtful since there are so many different use cases this PR touches due to the refactoring. I have just tested the most straight-forward one and I hope that no bugs were introduced for others (e.g. Open Office). But I trust you on this and at least there are plenty of tests.
|
@lenhard I manually tested most use cases, where the code changes (except OpenOffice since I don't use it). What are our current plans for the 4.1 release? When do we want to release? If there is still some time before the release I would prefer if this PR is merged before the release. |
|
You may want to add the percent sign and the ampersand (&), which is problematic as it creates a problem with biber/biblatex |
| StringBuilder newKey = new StringBuilder(); | ||
| for (int i = 0; i < key.length(); i++) { | ||
| char c = key.charAt(i); | ||
| if (!Character.isWhitespace(c) && ("{}(),\\\"#~^':`".indexOf(c) == -1)) { |
There was a problem hiding this comment.
You should reeally extract this to a constant, same with th e one abvoe
|
Should this still go into 4.1? |
|
@tobiasdiez When you fix the checkstyle issues and conflicts then this PR can be merged. |
Fixes #3359. Colons and apostrophes are now removed from the generated key pattern.
I also refactored the key generator, mainly converting the static methods to instance methods.
gradle localizationUpdate?