Remove newlines in PDF annotations #3291
Conversation
stefan-kolb
force-pushed
the
rm-newline-annotation
branch
from
1a601b5 to
92cc156
Compare
stefan-kolb
added
the
status: ready-for-review
| @@ -106,7 +106,10 @@ private String parseContent(final String content) { | |||
| return ""; | |||
| } | |||
|
|
|||
| return content.trim(); | |||
| // remove line breaks | |||
| String processedContent = content.replace("\r\n", " ").replace("\n", " ").replace("\r", " "); | |||
There was a problem hiding this comment.
Can you please reuse the remove new lines formatter here, otherwise this is good to go.
There was a problem hiding this comment.
@tobiasdiez This does not work with our current conventions, i had it this way before...: Adhere to architecture tests do not use logic from within model
There was a problem hiding this comment.
Ok, thats again one of these cases where I don't like our strict architecture tests. One could argue that the Formatter can actually life in model as well, but I don't want to start a war here :-)
There was a problem hiding this comment.
You can use the field formatter it in the view model of the file annotation instead of in the actual model (i.e. the actual model contains the text as it was in the pdf, but it is shown without line breaks).
| @@ -24,7 +24,7 @@ public String getKey() { | |||
| public String format(String value) { | |||
| Objects.requireNonNull(value); | |||
|
|
|||
| return value.replace("\r\n", " ").replace("\n", " ").trim(); | |||
| return value.replace("\r\n", " ").replace("\n", " ").replace("\r", " ").trim(); | |||
There was a problem hiding this comment.
I would also add a special treatment for hyphenated words.
Algorithm sketch: If the last character before the line termination symbol is a minus, replace the minus and the line-terminator with an empty string.
"...Jab-\n"
"ref is cool!\n"
=>
"...JabRef is cool!"
There was a problem hiding this comment.
What about Key-Account-Manager?
There was a problem hiding this comment.
Well... bad luck? words with a dash are quite rare, at least less frequent than hyphens at line breaks.
This is a problem, where we cant make a good decision.
There was a problem hiding this comment.
Words with a dash are quite rare in English, maybe...
There was a problem hiding this comment.
We can just add your solution and see if someone complains. I guess leaving the chars as they are is also not a bad decision.
There was a problem hiding this comment.
We've discussed this in the DevCall: Please go for the solution suggested by @LinusDietz
|
@JabRef/developers Can someone do the localization update for me? The script doesn't work on my end 😢 |
|
@stefan-kolb I neither can run the script |
|
Well if #3184 was broken, we can just revert the merge. |
|
@stefan-kolb The localization script is working again. Can you merge master into this and then it should be ready to go. |
|
Merging now, as this was discussed already in the devcall. The failing test concerning freecite is unrelated and has been fixed in #3445. |
Resolves #3280