-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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.
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
Fix attach file does not relativize file path #4252
Conversation
…tive * upstream/master: fix log4j slf4j dependency
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I've only one small suggestion to make the code more readable.
@@ -128,4 +128,12 @@ public LinkedFile getNewLinkedFile() { | |||
return new LinkedFile(description.getValue(), link.getValue(), monadicSelectedExternalFileType.map(ExternalFileType::toString).getOrElse("")); | |||
} | |||
|
|||
private void relativizeFile() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right now, it is hard to tell from the outside on which path this methods operates. I would thus prefer if relativizeFile
would accept a path and return the relativized path (probably also rename to relativize[Path]
). Then in would look like link.set(relativize(path))
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already had considered this option as well, but seem to have forgotten about it ;)
* upstream/master: (33 commits) Fix display of checkboxes Fix Remove Spin comments Fix#3861_build_addneeded/deleteobsoleteproperties2 Fix#3861_build_addneeded/deleteobsoleteproperties Fix#3861_build Fix attach file does not relativize file path (#4252) Fix for issue koppor 254 (Auto cleanup url) (#4255) Fix#ReplaceString_ChangeConflict Fix#3861ReplaceString_solveConflict Fix#3861RS_conflicttest Fix#3861RS_conflicttest Remove changelog entries Fix#3861ReplaceString_checkStyle Fix#3861ReplaceString_Improvements Fix#3861ReplaceString_ContributeLog fix log4j slf4j dependency Update dependencies (#4251) Fix author list parser (#4169) (#4248) ReplaceStringMVVMPattern ... # Conflicts: # src/main/java/org/jabref/gui/BasePanel.java
Fixes #4241
Attached files were only made relative before if you hit the browse button