Improve things arround change detection#5770
Conversation
| public static ParserResult loadDatabase(Path fileToOpen, ImportFormatPreferences importFormatPreferences, FileUpdateMonitor fileMonitor) | ||
| throws IOException { | ||
| ParserResult result = new BibtexImporter(importFormatPreferences, fileMonitor).importDatabase(fileToOpen.toPath(), | ||
| ParserResult result = new BibtexImporter(importFormatPreferences, fileMonitor).importDatabase(fileToOpen, |
There was a problem hiding this comment.
Why did you change it from path to file?
There was a problem hiding this comment.
This question is asked, because java.nio.Path should be preferred over java.io.File. We are aware in the code that this causes some conversion issues, however, I think, we should keep to move forward here. (Is this article still the best google result? --> https://jaxenter.de/java-nio-file-zeitgemases-arbeiten-mit-dateien-2581)
There was a problem hiding this comment.
I actually converted it to a Path, the method now accepts a Path variable and not a File
| public static ParserResult loadDatabase(String name, ImportFormatPreferences importFormatPreferences, FileUpdateMonitor fileMonitor) { | ||
| File file = new File(name); | ||
| LOGGER.info("Opening: " + name); | ||
| LOGGER.debug("Opening: " + name); |
There was a problem hiding this comment.
Doesn't "Opening {}", name work? - I think, this is the style one does in Java, because otherwise the debug string is always calculated even if the statement is not set.
There was a problem hiding this comment.
Isn't that micro optimization?
koppor
left a comment
There was a problem hiding this comment.
Think, the changes need to be tried out.
"Someone" should write tests emulating external modifications on the file in a parallel thread (or are we already having those kinds of tests?)
|
I agree writing tests would be a good idea. However that requires major refactoring beforehand as the problematic code is still tightly coupled to the UI 😣 |
# By Tobias Diez (11) and others # Via GitHub (1) and Tobias Diez (1) * upstream/master: (29 commits) Improve things arround change detection (#5770) Revert "Update to most recent journal abbreviation list" (#5769) Various fixes to the dark theme (#5764) Bump mockito-core from 3.2.0 to 3.2.4 (#5760) Bump classgraph from 4.8.58 to 4.8.59 (#5761) Improve dependency update rules Update jpackage to build 27 (#5758) Persistent column sortorder (#5730) Fix medline fetcher/importer when using installer (#5752) New Crowdin translations (#5751) Bump byte-buddy-parent from 1.10.4 to 1.10.5 (#5750) Fix checkstyle Fix filename Update to most recent journal abbreviation list Remove obsolete string Revert "Switch back to development" Switch back to development Next development cycle Release 5.0-beta (#5684) Fix multiple entries allowed in crossref (issue #5284) (#5724) ... # Conflicts: # src/main/java/org/jabref/gui/collab/ChangeDisplayDialog.java # src/main/java/org/jabref/gui/collab/EntryChangeViewModel.java
Another shot at fixing #4877. The changes in this PR might be already sufficient, let's see...
No time for extended PR description, see commit messages for changes. Last commit contains the actual "fix", the other ones are cosmetic changes.