Skip to content

Make attached files relative to the file directory#4212

Merged
Siedlerchr merged 7 commits into
masterfrom
fixattachfile
Jul 25, 2018
Merged

Make attached files relative to the file directory#4212
Siedlerchr merged 7 commits into
masterfrom
fixattachfile

Conversation

@Siedlerchr

@Siedlerchr Siedlerchr commented Jul 17, 2018

Copy link
Copy Markdown
Member

Fixes #4201


  • Change in CHANGELOG.md described
  • Tests created for changes
  • Manually tested changed features in running JabRef
  • Screenshots added in PR description (for bigger UI changes)
  • Ensured that the git commit message is a good one
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)

Fixes #4201
Open files with default OS application if no External file type is present
@Siedlerchr Siedlerchr changed the title Fixattachfile Make attached files relative to the file directory Jul 17, 2018
@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jul 17, 2018

@tobiasdiez tobiasdiez left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would have preferred if the bug is fixed via refactoring. Other remarks are just minor nitpicking.

dialogService.showFileOpenDialog(fileDialogConfiguration).ifPresent(newFile -> {
LinkedFile newLinkedFile = new LinkedFile("", newFile.toString(), "");

Path relativePath = FileUtil.shortenFileName(newFile, panel.getBibDatabaseContext().getFileDirectoriesAsPaths(Globals.prefs.getFileDirectoryPreferences()));

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

In my opinion, this code should be refactored to use the same code as the same action in the entry editor. Otherwise, we will again run into a similar problem where the two actions are different in behavior.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Changed it


public LinkedFile getNewLinkedFile() {
return new LinkedFile(description.getValue(), link.getValue(), selectedExternalFileType.getValue().toString());
return new LinkedFile(description.getValue(), link.getValue(), selectedExternalFileType.getValue() == null ? null : selectedExternalFileType.getValue().toString());

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

As we wanted to get ride of null's, I don't like this change. Maybe we should use a MonadicBinding (optional binding) and use an empty string as file type?

NATIVE_DESKTOP.openFileWithApplication(filePath, application);
}
} else {
NATIVE_DESKTOP.openFile(filePath, null);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add a short comment explaining what is done here.

* upstream/master:
  Update dependencies (#4231)
  Update to latest release of richtextfx (#4213)
  Fix fetcher tests (#4216)
  single line text fields (#4138)
  Make it easier to rename and move files (#4200)
  Fix that swing dialogs are hidden behind main window (#4205)
  Fixed: #4166 add move & rename file (#4217)

# Conflicts:
#	CHANGELOG.md
* upstream/master:
  Fix isbn chimbori test (#4234)
  Rewrite web search pane in JavaFX (#4203)
add monadic binding

@tobiasdiez tobiasdiez left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me now.

@Siedlerchr Siedlerchr merged commit 077fdac into master Jul 25, 2018
@Siedlerchr Siedlerchr deleted the fixattachfile branch July 25, 2018 14:50
Siedlerchr added a commit that referenced this pull request Jul 26, 2018
* upstream/master: (47 commits)
  Make attached files relative to the file directory (#4212)
  execute set visible in swing thread to avoid blocking
  Fix isbn chimbori test (#4234)
  Rewrite web search pane in JavaFX (#4203)
  Update dependencies (#4231)
  Update to latest release of richtextfx (#4213)
  Fix fetcher tests (#4216)
  single line text fields (#4138)
  Make it easier to rename and move files (#4200)
  Fix that swing dialogs are hidden behind main window (#4205)
  Fixed: #4166 add move & rename file (#4217)
  update gradle plugins and gradlen to 4.9
  Update dependencies
  Remove unnecessary look and feel migration (#4204)
  Revert threading fix since this sometimes lead to freezes...
  Update year
  Fix a few more threading exceptions
  Refactor
  Convert CiteSeerX fetcher to new infrastructure (#4185)
  Make global font size customizable (#4178)
  ...

# Conflicts:
#	src/main/java/org/jabref/gui/BasePanel.java
#	src/main/java/org/jabref/gui/JabRefFrame.java
#	src/main/java/org/jabref/gui/mergeentries/MergeEntries.java
#	src/main/resources/l10n/JabRef_en.properties
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants