Add 'Open example library' button to Welcome Tab#13068
Conversation
| taskExecutor).openFiles(List.of(tmpFile)); | ||
| tmpFile.toFile().deleteOnExit(); | ||
| } catch (IOException ex) { | ||
| throw new RuntimeException(ex); |
There was a problem hiding this comment.
Using exceptions for control flow is not recommended. Exceptions should be used for exceptional states, not for normal control flow.
There was a problem hiding this comment.
Just add a logger statement here or show an errror dialog
dialogService.showEror...
| } | ||
| } | ||
| } catch (IOException ex) { | ||
| throw new RuntimeException(ex); |
koppor
left a comment
There was a problem hiding this comment.
tmp file is not the right approach.
| if (in != null) { | ||
| Path tmpFile = Files.createTempFile("example-library", ".bib"); | ||
| try { | ||
| Files.copy(in, tmpFile, StandardCopyOption.REPLACE_EXISTING); |
There was a problem hiding this comment.
Wrong approach.
- Create a new library
- Fill it with contents of
Chocolate.bib. For that, you need to useorg.jabref.logic.importer.fileformat.BibtexParser#BibtexParser(org.jabref.logic.importer.ImportFormatPreferences). Then you can call org.jabref.model.database.BibDatabase#insertEntries(java.util.List<org.jabref.model.entry.BibEntry>) - similar toaddExampleButton.setOnAction(...
There was a problem hiding this comment.
Please try to keep the file in jablib. Report back if you cannot load it.
- Passes each entry in "Chocolate.bib" - Creates an empty database object - Inserts the entries into the database - Wraps the database in BibDatabaseContext - Creates a new library tab with the data loaded from "Chocolate.bib"
|
@khushp3 please do not request a review before you have addressed the issues risen by the tests |
| BibtexParser bibtexParser = new BibtexParser(preferences.getImportFormatPreferences(), fileUpdateMonitor); | ||
| List<BibEntry> entries = bibtexParser.parseEntries(in); | ||
|
|
||
| BibDatabase database = new BibDatabase(); | ||
| database.insertEntries(entries); | ||
|
|
||
| BibDatabaseContext databaseContext = new BibDatabaseContext(database); |
There was a problem hiding this comment.
Seeing this, there is a more simpler way:
| BibtexParser bibtexParser = new BibtexParser(preferences.getImportFormatPreferences(), fileUpdateMonitor); | |
| List<BibEntry> entries = bibtexParser.parseEntries(in); | |
| BibDatabase database = new BibDatabase(); | |
| database.insertEntries(entries); | |
| BibDatabaseContext databaseContext = new BibDatabaseContext(database); | |
| reader = Importer.getReader(inputStream); | |
| BibtexParser bibtexParser = new BibtexParser(preferences.getImportFormatPreferences(), fileUpdateMonitor); | |
| ParserResult result = bibtexParser.parse(reader); | |
| BibDatabaseContext databaseContext = result.getDatabaseContext(); |
I found simlar code in org.jabref.logic.importer.fileformat.BibtexParser
| fileHistoryMenu.getItems().addListener((ListChangeListener<MenuItem>) _ -> updateWelcomeRecentLibraries()); | ||
| fileHistoryMenu.getItems().addListener((ListChangeListener<MenuItem>) e -> updateWelcomeRecentLibraries()); |
There was a problem hiding this comment.
Is this change AI generated? Please keep _
| import org.jabref.model.entry.BibEntryTypesManager; | ||
| import org.jabref.model.util.FileUpdateMonitor; | ||
|
|
||
| import static org.jabref.gui.edit.automaticfiededitor.AbstractAutomaticFieldEditorTabViewModel.LOGGER; |
There was a problem hiding this comment.
Copy and paste issue - see https://devdocs.jabref.org/code-howtos/logging.html how to initialize the logger
| Reader reader; | ||
| reader = Importer.getReader(in); |
There was a problem hiding this comment.
Merge in one line; you can even move it up into the try braces
| taskExecutor).execute()); | ||
|
|
||
| return createVBoxContainer(startLabel, newLibraryLink, openLibraryLink); | ||
| Hyperlink openExampleLibraryLink = new Hyperlink(Localization.lang("Open example library")); |
There was a problem hiding this comment.
- The string should be
New example library - Rename "New library" to "New empty library"
- position it one above, so that the order is
- New empty library
- New example library
- Open library
- Changed string names
…d JabRef_en.properties - Correctly imported LOGGER
| RELEVANCE(Localization.lang("Relevance"), IconTheme.JabRefIcons.RELEVANCE), | ||
| RELEVANT(Localization.lang("Toggle relevance"), IconTheme.JabRefIcons.RELEVANCE), | ||
| NEW_LIBRARY(Localization.lang("New library"), IconTheme.JabRefIcons.NEW), | ||
| NEW_LIBRARY(Localization.lang("New empty library"), IconTheme.JabRefIcons.NEW), |
There was a problem hiding this comment.
The change from 'New library' to 'New empty library' should be consistent with other strings and grouped semantically. Ensure that similar actions use consistent terminology.
|
I fixed the small test issues (wrong variable order and translation strings) Now, it is good to go. Thank you for the efforts! |
|
@trag-bot didn't find any issues in the code! ✅✨ |
1 similar comment
|
@trag-bot didn't find any issues in the code! ✅✨ |
Closes #13014
Mandatory checks
CHANGELOG.mddescribed in a way that is understandable for the average user (if change is visible to the user)