Auto relativize PDF file paths when selecting "Search for unlinked local files" #12088

u7676493 · 2024-10-26T03:34:46Z

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

Modified ImportHandler to use different arguments for importPDFContent(), accounting for the changes made to ExternalFilesContentImporter
ImportHandler starts the execution of the logic of importing a PDF file.
Modified ExternalFilesContentImporter to include imports for BibDatabaseContext and FilePreferences classes as well as different arguments for importPDFContent() and importDatabase(), both related to the changes made to PdfMergeMetadataImporter
Modified PdfMergeMetadataImporter to add an alternate importDatabase() function with different arguments, using the BibDatabaseContext.getFileDirectories(FilePreferences) function to get all file directories and relativize the file path using the FileUtil.relativize(Path, List<Path>) function, before passing it into the default override of the importDatabase(Path) function from the Importer class.
PdfMergeMetadataImporter handles all of the actual logic relating to importing a PDF file.
The new importDatabase() function has three arguments: (Path file, BibDatabaseContext context, FilePreferences preferences)
Added a WIP test in PdfMergeMetadataImporter

Please provide me with any feedback you can! I haven't included any changes in documentation yet because this is still a WIP.

Mandatory checks

I own the copyright of the code submitted and I licence it under the MIT license
Change in CHANGELOG.md described in a way that is understandable for the average user (if applicable)
Tests created for changes (if applicable)
Manually tested changed features in running JabRef (always required)
Screenshots added in PR description (for UI changes)
Checked developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

…lativized.

subhramit · 2024-10-26T10:46:43Z

src/main/java/org/jabref/gui/externalfiles/ImportHandler.java

@@ -118,7 +118,7 @@ public List<ImportFilesResultItemViewModel> call() {

                    try {
                        if (FileUtil.isPDFFile(file)) {
-                            var pdfImporterResult = contentImporter.importPDFContent(file);
+                            var pdfImporterResult = contentImporter.importPDFContent(file, bibDatabaseContext, filePreferences);


Can you change this to the strict returned type here?

subhramit · 2024-10-26T10:52:41Z

src/main/java/org/jabref/logic/importer/fileformat/PdfMergeMetadataImporter.java

+     * @param context     Everything related to the BIB file
+     * @param preferences Preferences for linked files


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.

subhramit · 2024-10-26T10:54:01Z

src/test/java/org/jabref/logic/importer/fileformat/PdfMergeMetadataImporterTest.java

+        // Set up results
+        List<BibEntry> result = importer.importDatabase(file, database, preferences).getDatabase().getEntries();
+
+        // Set up expected value
+        BibEntry expected = new BibEntry(StandardEntryType.InProceedings)


Remove these two comments (redundant).

u7676493 · 2024-10-26T20:55:44Z

I have just added a commit that follows up on all of the requested changes so far.

Removes unnecessary comments

koppor

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!

koppor · 2024-10-26T23:20:44Z

src/main/java/org/jabref/logic/externalfiles/ExternalFilesContentImporter.java

@@ -17,9 +19,9 @@ public ExternalFilesContentImporter(ImportFormatPreferences importFormatPreferen
        this.importFormatPreferences = importFormatPreferences;
    }

-    public ParserResult importPDFContent(Path file) {
+    public ParserResult importPDFContent(Path file, BibDatabaseContext context, FilePreferences preferences) {


Name the preferences variable filePreferences - also in the method importDatabase

…FContent

It now correctly states which function is called to produce the return value

koppor · 2024-10-27T11:12:40Z

@u7676493 I already added it to the merge queue. You updated. Do you want to do any more changes?

koppor · 2024-10-27T11:13:54Z

src/main/java/org/jabref/logic/importer/fileformat/PdfMergeMetadataImporter.java

+        List<Path> directories = context.getFileDirectories(filePreferences);
+
+        if (!directories.isEmpty()) {
+            filePath = FileUtil.relativize(filePath, directories);


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.

I have just added a commit that does that

Removed unnecessary if statement

subhramit

Looks good. Adding back to merge queue.

u7676493 added 2 commits October 26, 2024 13:36

When using "Find Unlinked Files" to import files, they will now be re…

d798ddf

…lativized.

Removes debug lines

eb8722b

subhramit requested changes Oct 26, 2024

View reviewed changes

Removes debug lines

4484154

Uses the strict return type on pdfImporterResult

8ea772c

Removes unnecessary comments

koppor marked this pull request as ready for review October 26, 2024 23:19

koppor requested changes Oct 26, 2024

View reviewed changes

u7676493 added 3 commits October 27, 2024 10:33

Renamed preferences to filePreferences in importDatabase and importPD…

5865d20

…FContent

Merge remote-tracking branch 'origin/auto-relativize-file-paths'

3aeecd2

Update CHANGELOG.md to include changes

b57bf68

u7676493 changed the title ~~[WIP] Auto relativize PDF file paths when selecting "Find Unlinked Files"~~ Auto relativize PDF file paths when selecting "Find Unlinked Files" Oct 26, 2024

u7676493 changed the title ~~Auto relativize PDF file paths when selecting "Find Unlinked Files"~~ Auto relativize PDF file paths when selecting "Search for unlinked local files" Oct 26, 2024

Merge branch 'main' into auto-relativize-file-paths

019500f

koppor previously approved these changes Oct 26, 2024

View reviewed changes

koppor enabled auto-merge October 26, 2024 23:46

u7676493 added 2 commits October 27, 2024 16:18

Update description of PdfMergeMetadataImporter.importDatabase

de348cc

It now correctly states which function is called to produce the return value

Merge remote-tracking branch 'origin/auto-relativize-file-paths'

2e01a24

auto-merge was automatically disabled October 27, 2024 05:18
Head branch was pushed to by a user without write access

u7676493 dismissed koppor’s stale review via 2e01a24 October 27, 2024 05:19

Merge branch 'main' into auto-relativize-file-paths

936787f

koppor reviewed Oct 27, 2024

View reviewed changes

Update PdfMergeMetadataImporter.importDatabase

098c713

Removed unnecessary if statement

subhramit previously approved these changes Oct 27, 2024

View reviewed changes

Merge branch 'main' into auto-relativize-file-paths

ae55866

subhramit dismissed their stale review via ae55866 October 27, 2024 22:58

subhramit enabled auto-merge October 27, 2024 22:58

subhramit approved these changes Oct 27, 2024

View reviewed changes

subhramit added this pull request to the merge queue Oct 27, 2024

Merged via the queue into JabRef:main with commit b9f3951 Oct 27, 2024
23 checks passed

koppor mentioned this pull request Nov 10, 2024

Path should be relative after import at "Find Unlinked Files" JabRef/jabref-koppor#549

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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" #12088

u7676493 commented Oct 26, 2024 •

edited

Loading

subhramit Oct 26, 2024

subhramit Oct 26, 2024 •

edited

Loading

subhramit Oct 26, 2024 •

edited

Loading

u7676493 commented Oct 26, 2024

koppor left a comment

koppor Oct 26, 2024

koppor commented Oct 27, 2024

koppor Oct 27, 2024

u7676493 Oct 27, 2024

subhramit left a comment

		* @param context Everything related to the BIB file
		* @param preferences Preferences for linked files

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" #12088

Conversation

u7676493 commented Oct 26, 2024 • edited Loading

List of changes

Mandatory checks

subhramit Oct 26, 2024

Choose a reason for hiding this comment

subhramit Oct 26, 2024 • edited Loading

Choose a reason for hiding this comment

subhramit Oct 26, 2024 • edited Loading

Choose a reason for hiding this comment

u7676493 commented Oct 26, 2024

koppor left a comment

Choose a reason for hiding this comment

koppor Oct 26, 2024

Choose a reason for hiding this comment

koppor commented Oct 27, 2024

koppor Oct 27, 2024

Choose a reason for hiding this comment

u7676493 Oct 27, 2024

Choose a reason for hiding this comment

subhramit left a comment

Choose a reason for hiding this comment

u7676493 commented Oct 26, 2024 •

edited

Loading

subhramit Oct 26, 2024 •

edited

Loading

subhramit Oct 26, 2024 •

edited

Loading