Skip to content

Opens the directory of the currently edited file when opening a new file#4106

Merged
Siedlerchr merged 5 commits into
JabRef:masterfrom
JavierMF:fix-for-issue-4097
Jun 12, 2018
Merged

Opens the directory of the currently edited file when opening a new file#4106
Siedlerchr merged 5 commits into
JabRef:masterfrom
JavierMF:fix-for-issue-4097

Conversation

@JavierMF

@JavierMF JavierMF commented Jun 5, 2018

Copy link
Copy Markdown
Contributor

Opens the directory of the currently edited file when opening a new file
Fixes #4097


  • 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?)

@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.

Thanks a lot for your contribution! The code looks good to me. Could you please add a short remark to the Changelog.md explaining the change.

@JavierMF

JavierMF commented Jun 6, 2018

Copy link
Copy Markdown
Contributor Author

Done!

return getWorkingDirectoryPath();
} else {
Optional<Path> databasePath = frame.getCurrentBasePanel().getBibDatabaseContext().getDatabasePath();
return !databasePath.isPresent() ? getWorkingDirectoryPath() : databasePath.get().getParent();

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.

Just a minor improvement, you can use Optionals orElse here, if the value is present it is returned, otherwise the value from the orElse will be returned

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you! The point is that if the value is present I don't want to return the value itself, but the .getParent(). And, int the other branch, I can not apply .getParent() to the value saved in the WORKING_DIRECTORY. I don't have lot of experience with optionals and don't know how to use the orElse in this situation.

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.

This should do the trick: databasePath.map(p -> p.getParent()).orElse(getWorkingDirectoryPath())

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Cool! I was not aware map() could be used with optionals! Thank you.

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.

Hi,

that should be possible with Optional.map: (You can chain optionals similar to streams, here is a nice overview: https://www.nurkiewicz.com/2013/08/optional-in-java-8-cheat-sheet.html )

This will transform the value by executing getParent if the value is present, otherwise it would return the workingDir path.

return databasePath.map(Path::getParent).orElse(getWorkingDirectoryPath?)

PS: Sorry that I reply today, I saw that I did a review using githubs feature, but did not submit the review 🤦‍♂️ )

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thank you again, @Siedlerchr. I just applied and pushed the refactor proposed by @koppor before seeing your 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.

It's essentially the same, but using method references looks nicer ;)

@Siedlerchr Siedlerchr 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.

Thanks again for your contribution! 👍

@Siedlerchr Siedlerchr merged commit db9e4e5 into JabRef:master Jun 12, 2018
@JavierMF JavierMF deleted the fix-for-issue-4097 branch June 12, 2018 20:20
Siedlerchr added a commit that referenced this pull request Jun 13, 2018
* upstream/master:
  Extract v4.x changelog (#4125)
  Saves and reloads window state on close and on open (#4124)
  Fix convert to bibtex moves contents of the file field (#4123)
  Opens the directory of the currently edited file when opening a new file (#4106)
  Show Citation style also in entry preview in preferences (#4121)
  Mac graphics (#4074)
  The textarea in the PersonsEditor is focused when the field is focused (#4112)
Siedlerchr added a commit that referenced this pull request Jun 15, 2018
* upstream/master: (22 commits)
  fix failing architecture test by making test class public again migrate architecture test to junit jupiter
  Fix build... (#4128)
  fix checkstyle after merge
  Migrate Review field in entry preview to comment (#4100)
  [WIP] Split push to applications in logic and gui (#4110)
  Fix checkstyle
  Fix #4115: Don't report journal name as abbreviated when full name = abbreviated name (#4116)
  Use <kbd>in changelog
  Groups right click (#4061)
  Fix open thread prevents shutdown (#4111)
  Extract v4.x changelog (#4125)
  Saves and reloads window state on close and on open (#4124)
  Fix convert to bibtex moves contents of the file field (#4123)
  Opens the directory of the currently edited file when opening a new file (#4106)
  Don't run on Swing thread
  Properly shutdown telemetry client
  Code cleanup
  Remove Swing stuff (L&F, font customization)
  Properly shutdown JabRef (not with System.exit)
  Replace swing clipboard with JavaFX one
  ...

# Conflicts:
#	src/main/java/org/jabref/gui/BasePanel.java
#	src/main/java/org/jabref/gui/JabRefFrame.java
#	src/main/java/org/jabref/gui/maintable/MainTable.java
#	src/main/java/org/jabref/gui/search/SearchResultFrame.java
#	src/main/java/org/jabref/preferences/JabRefPreferences.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants