Fixed #1264 #1581
Fixed #1264 #1581
Conversation
oscargus
added
bug
| for (Character character : chars) { | ||
| result = result.replace(character.toString(), | ||
| HTMLUnicodeConversionMaps.UNICODE_LATEX_CONVERSION_MAP.get(character)); | ||
| Set<String> strings = HTMLUnicodeConversionMaps.UNICODE_LATEX_CONVERSION_MAP.keySet(); |
There was a problem hiding this comment.
Please find a more speaking variable name as "strings" here and below in the loop
e.g unicode_strings or whatever..
|
@stefan-kolb as it was only |
|
@oscargus It was only the \chi, yes because \c is handled as special character. If my test from the PR pass we can merge my test PR into your branch and then merge your fix 😄 |
|
In general, I would like tests for this PR. |
|
My PR #1464 includes tests for the bug. |
|
@stefan-kolb There is a @tobiasdiez: I was considering that. However, the main feature is adding support for accented unicode characters and they are a hassle to type properly... Nor will they make much sense except lots of comments. For example, spot the difference between ä and ä... |
oscargus
force-pushed
the
latextounicodefix
branch
from
2a4a045 to
4addbcf
Compare
|
@oscargus Can you just cherry-pick my commit from the PR and you have your first test 😄 |
|
I added some tests now. @stefan-kolb I'm afraid that is a bit out of my comfort/knowledge zone... My solution would be to almost simultaneously merge both PRs. :-) |
|
@oscargus Then just copy my tests and close the PR 😄 |
|
@stefan-kolb That should be within grasp. :-) |
| @@ -54,7 +54,7 @@ | |||
|
|
|||
|
|
|||
|
|
|||
| public static final String SPECIAL_COMMAND_CHARS = "\"`^~'c="; | |||
| public static final String SPECIAL_COMMAND_CHARS = "\"`^~'=.|"; // "\"`^~'c=" | |||
There was a problem hiding this comment.
Can you add a comment describing this string?
|
LGTM thanks for the quick solution! :) |
|
LGTM 👍 |
| @@ -31,6 +31,7 @@ | |||
| public class LatexToUnicodeFormatter implements LayoutFormatter, Formatter { | |||
|
|
|||
| private static final Map<String, String> CHARS = HTMLUnicodeConversionMaps.LATEX_UNICODE_CONVERSION_MAP; | |||
| private static final Map<String, String> ACCENTS = HTMLUnicodeConversionMaps.UNICODE_ESCAPED_ACCENTS; | |||
There was a problem hiding this comment.
@JabRef/developers Does java copies here the whole map or just point to the original one? If the map is copied, then one should inline CHARS and ACCENTS
There was a problem hiding this comment.
It doesn't matter, as this is a kind of constant which gets replaced by the containing value on compile time, directly in the byte code.
In general, java is by value, it copies just the object reference (pointer).
http://stackoverflow.com/questions/40480/is-java-pass-by-reference-or-pass-by-value
|
LGTM 👍 just some minor remarks about naming variables and tests |
oscargus
force-pushed
the
latextounicodefix
branch
from
9bda49f to
1148021
Compare
I finally got around to understand the reasons for #1264 (and parts of #1464). It turned out that commands starting with c was handled in a special way and that
{\v{S}}was for unknown reasons used as "line tabulation set".As a result, not only does JabRef now render
\v{S}and\chicorrectly, it also renders combining accents, see screen shot.