Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes making paths of linked files relative (web urls will not be touched anymore) #5879

Merged
merged 3 commits into from
Jan 29, 2020
Merged

Fixes making paths of linked files relative (web urls will not be touched anymore) #5879

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8ad6f23
Fixes #5861
Jan 27, 2020
f033dbf
changelog updated
Jan 28, 2020
4adfdc5
refactoring: use existing method isOnlineLink() for checking whether …
Jan 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
- We fixed an issue where the ampersand character wasn't rendering correctly on previews. [#3840](https://github.com/JabRef/jabref/issues/3840)
- We fixed an issue where an erroneous "The library has been modified by another program" message was shown when saving. [#4877](https://github.com/JabRef/jabref/issues/4877)
- We fixed an issue where the file extension was missing after downloading a file (we now fall-back to pdf). [#5816](https://github.com/JabRef/jabref/issues/5816)
- We fixed an issue where cleaning up entries broke web URLs, if "Make paths of linked files relative (if possible)" was enabled, which resulted in various other issues subsequently. [#5861](https://github.com/JabRef/jabref/issues/5861)

### Removed
- Ampersands are no longer escaped by default in the `bib` file. If you want to keep the current behaviour, you can use the new "Escape Ampersands" formatter as a save action.
Expand Down
32 changes: 28 additions & 4 deletions src/main/java/org/jabref/logic/cleanup/RelativePathsCleanup.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,17 @@ public List<FieldChange> cleanup(BibEntry entry) {

for (LinkedFile fileEntry : fileList) {
String oldFileName = fileEntry.getLink();
String newFileName = FileUtil
.relativize(Paths.get(oldFileName), databaseContext.getFileDirectoriesAsPaths(filePreferences))
.toString();

String newFileName = null;
if (isWebUrl(oldFileName)) {
// let web url untouched
newFileName = oldFileName;
}
else {
// only try to transform local file path to relative one
newFileName = FileUtil
.relativize(Paths.get(oldFileName), databaseContext.getFileDirectoriesAsPaths(filePreferences))
.toString();
}
LinkedFile newFileEntry = fileEntry;
if (!oldFileName.equals(newFileName)) {
newFileEntry = new LinkedFile(fileEntry.getDescription(), newFileName, fileEntry.getFileType());
Expand All @@ -57,4 +64,21 @@ public List<FieldChange> cleanup(BibEntry entry) {
return Collections.emptyList();
}

/**
* Checks, if the given file path is a well-formed web url
*
* @param filePath file path to check
* @return <code>true</code>, if the given file path is a web url, <code>false</code> otherwise
*/
private static boolean isWebUrl(String filePath)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The class LinkedFile already contains a method that does this job:

jabref/src/main/java/org/jabref/model/entry/LinkedFile.java

Lines 162 to 164 in 8520833

public boolean isOnlineLink() {
return isOnlineLink(link.get());
}

Can you please use it. I would also suggest to add the normalization trim().toLowerCase() there, which I find a nice addition.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concerning isOnlineLink(): I would recommend removing "toCheck.contains("www.")", because a path to a local file might contain this pattern coincidentally. What do you say?

{
if (filePath == null) {
return false;
}
String normalizedFilePath = filePath.trim().toLowerCase();
if (normalizedFilePath.startsWith("http://") || normalizedFilePath.startsWith("https://")) { // INFO: can be extended, if needed
return true;
}
return false;
}
}