Auto relativize PDF file paths when selecting "Search for unlinked local files"#12088
Auto relativize PDF file paths when selecting "Search for unlinked local files"#12088subhramit merged 13 commits intoJabRef:mainfrom
Conversation
| try { | ||
| if (FileUtil.isPDFFile(file)) { | ||
| var pdfImporterResult = contentImporter.importPDFContent(file); | ||
| var pdfImporterResult = contentImporter.importPDFContent(file, bibDatabaseContext, filePreferences); |
There was a problem hiding this comment.
Can you change this to the strict returned type here?
| * @param context Everything related to the BIB file | ||
| * @param preferences Preferences for linked files |
There was a problem hiding this comment.
146 - To be more precise, context is the metadata for the library. However, it's a common JabRef parameter, so can remove it.
147 - Can remove as well, as the type of the parameter makes the description redundant.
| // Set up results | ||
| List<BibEntry> result = importer.importDatabase(file, database, preferences).getDatabase().getEntries(); | ||
|
|
||
| // Set up expected value | ||
| BibEntry expected = new BibEntry(StandardEntryType.InProceedings) |
There was a problem hiding this comment.
Remove these two comments (redundant).
|
I have just added a commit that follows up on all of the requested changes so far. |
Removes unnecessary comments
koppor
left a comment
koppor
left a comment
There was a problem hiding this comment.
Tried it out. Works.
Small naming comment - otherwise code looks good. No need for additional tests. - Code is pretty similar with #10582 - which we also reviewed back then. - The PR was abandoned, thank you for porting it to the most recent version of JabRef.
Please add CHANGELOG.md entry and then it should be good to go!
| } | ||
|
|
||
| public ParserResult importPDFContent(Path file) { | ||
| public ParserResult importPDFContent(Path file, BibDatabaseContext context, FilePreferences preferences) { |
There was a problem hiding this comment.
Name the preferences variable filePreferences - also in the method importDatabase
u7676493
changed the title
u7676493
changed the title
It now correctly states which function is called to produce the return value
Head branch was pushed to by a user without write access
|
@u7676493 I already added it to the merge queue. You updated. Do you want to do any more changes? |
| List<Path> directories = context.getFileDirectories(filePreferences); | ||
|
|
||
| if (!directories.isEmpty()) { | ||
| filePath = FileUtil.relativize(filePath, directories); |
There was a problem hiding this comment.
Doesn't that method also have a check for emptyness? Then, this call can be moved out of the if and the jf be removed.
There was a problem hiding this comment.
I have just added a commit that does that
Removed unnecessary if statement
subhramit
left a comment
subhramit
left a comment
There was a problem hiding this comment.
Looks good. Adding back to merge queue.
3 participants
Fixes koppor#549
It updates the functionality of the "Search for unlinked local files" import option - it now automatically relativizes the file paths for PDF files.
List of changes
ImportHandlerto use different arguments forimportPDFContent(), accounting for the changes made toExternalFilesContentImporterImportHandlerstarts the execution of the logic of importing a PDF file.ExternalFilesContentImporterto include imports forBibDatabaseContextandFilePreferencesclasses as well as different arguments forimportPDFContent()andimportDatabase(), both related to the changes made toPdfMergeMetadataImporterPdfMergeMetadataImporterto add an alternateimportDatabase()function with different arguments, using theBibDatabaseContext.getFileDirectories(FilePreferences)function to get all file directories and relativize the file path using theFileUtil.relativize(Path, List<Path>)function, before passing it into the default override of theimportDatabase(Path)function from theImporterclass.PdfMergeMetadataImporterhandles all of the actual logic relating to importing a PDF file.The new
importDatabase()function has three arguments:(Path file, BibDatabaseContext context, FilePreferences preferences)PdfMergeMetadataImporterPlease provide me with any feedback you can! I haven't included any changes in documentation yet because this is still a WIP.
Mandatory checks
CHANGELOG.mddescribed in a way that is understandable for the average user (if applicable)