Use JDK 15 text blocks to improve injected languages readability#8874
Conversation
This reverts commit 376760c.
- Converted automatically by IntelliJ
- All tests pass. - I didn't modify newline tests. - text blocks uses '\n' as the default newline character for all OS's, that is why I replaced all '\n' by OS.NEWLINE.
|
Hello, @koppor. Thank you for the review. Do you think all strings should be converted at once in this PR, or should we take an incremental approach and spread the conversion across multiple PRs? I'm asking because I'm concerned that if this PR gets large and something goes wrong later, it will be difficult to track down what changes caused it. |
| + " \"number\":\"5\",\r\n" + " \"startingPage\":\"550\",\r\n" | ||
| + " \"url\":\"http://dx.doi.org/10.1007/BF01201962\",\"copyright\":\"©1992 Rapid Communications of Oxford Ltd.\"\r\n" | ||
| + " }"; | ||
| String jsonString = """ |
There was a problem hiding this comment.
This looks odd? Does that still work with \rn
There was a problem hiding this comment.
Yes, the compiler will append \n at the end of each line of a text block, so it will become \r\n. If we want to keep the \r\n character, we can escape the newline character by adding a backslash at the end of each line, as shown here \r\n\ .
There was a problem hiding this comment.
This also works because of following .gitattributes content:
Line 10 in 8b30960
|
I would not want to convert all of them now. I especially worry about cases where we have different line endings. And I think it's not that important now. However, I think the uses case for the embeded javascript syntax is a good example! |
|
@Siedlerchr I agree; converting strings with different line endings in |
+1 for incremental rewrites. I would touch that part of the code only where you intend to work on the tests. Otherwise, too much energy is spent in "fixing" have-worked tests. |
* upstream/main: Add an importer for Citavi backup files (#8848) Reviewdoc: Comment on PRs (#8878) Squashed 'buildres/csl/csl-styles/' changes from 649aac4..e740261 Use JDK 15 text blocks to improve injected languages readability (#8874) Fix fetcher tests (#8877) Fix #8390 by allowing multiple group deletion for Remove groups > Kee… (#8875) Add restart warning on SSL configuration change (#8871) Update to lucene 9.2 (#8868) Fix for removing several groups deletes only one of them (#8801) Disable Write XMP Button in General tab of Entry-Editor when action is in progress (#8728) Bump jsoup from 1.14.3 to 1.15.1 (#8864) Bump unirest-java from 3.13.8 to 3.13.10 (#8869) Bump unoloader from 7.3.2 to 7.3.3 (#8863) Bump pascalgn/automerge-action from 0.15.2 to 0.15.3 (#8860) Bump classgraph from 4.8.146 to 4.8.147 (#8861) Bump mockito-core from 4.5.1 to 4.6.0 (#8862) Lucence dir checkers should only delete lucence dirs (#8854) Update README.md (#8858) Update adr.md Update adr.md
For the preview panel, IntelliJ was able to detect and convert one string automatically but I had to manually convert the rest, so I may have made mistakes. However, I did test the preview panel manually and everything seems to work properly. Using IntelliJ's Language injection feature to ensure the JavaScript and HTML code is syntactically correct was quite helpful. In addition, I found around 16 potential files that can benefit from text blocks and thus might be handled in this PR (found it by searching for this string in java files '\n" +').
This is how injected JavaScript looks like in the IDE
Notes
\nas its line separator that's why I usedreplaceAll('\n', OS.NEWLINE)inBibEntryWriterTest.CHANGELOG.mddescribed in a way that is understandable for the average user (if applicable)