Skip to content

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

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

Merged
merged 5 commits into from
Jun 12, 2018

Conversation

JavierMF
Copy link
Contributor

@JavierMF JavierMF commented Jun 5, 2018

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

Copy link
Member

@tobiasdiez tobiasdiez left a comment

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
Copy link
Contributor Author

JavierMF commented Jun 6, 2018

Done!

return getWorkingDirectoryPath();
} else {
Optional<Path> databasePath = frame.getCurrentBasePanel().getBibDatabaseContext().getDatabasePath();
return !databasePath.isPresent() ? getWorkingDirectoryPath() : databasePath.get().getParent();
Copy link
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
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
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
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
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
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
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 ;)

Copy link
Member

@Siedlerchr Siedlerchr left a comment

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