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.

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 #6357: File directory #6377

Merged
merged 4 commits into from
Apr 30, 2020
Merged

Fixes #6357: File directory #6377

merged 4 commits into from
Apr 30, 2020

Conversation

tobiasdiez
Copy link
Member

@tobiasdiez tobiasdiez commented Apr 29, 2020

Bug was introduced in 1b03f03. Sorry @calixtus for suspecting you 👼.

Also a bit of code cleanup is included. For example, I tried to minimize using strings and favored Paths instead.

  • Change in CHANGELOG.md described (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

Bug was introduced in 1b03f03.
@tobiasdiez tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Apr 29, 2020
Copy link
Member

@calixtus calixtus left a comment

Choose a reason for hiding this comment

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

LGTM, except one concern about Paths::get

return Optional.empty();
} else {
return Optional.of(Paths.get(mainFileDirectory));
Copy link
Member

Choose a reason for hiding this comment

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

JavaDoc indicates, that Paths::get will be deprecated soon. Use Path::of instead.
https://docs.oracle.com/en/java/javase/14/docs/api/java.base/java/nio/file/Paths.html

fileDirectories);
void getFileDirectoriesWithEmptyDbParent() {
BibDatabaseContext database = new BibDatabaseContext();
database.setDatabasePath(Paths.get("biblio.bib"));
Copy link
Member

Choose a reason for hiding this comment

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

Path::of and also below

@calixtus
Copy link
Member

calixtus commented Apr 30, 2020

Sorry @calixtus for suspecting you 👼.

Forgiven and forgotten. You owe me one. 😉

@tobiasdiez
Copy link
Member Author

Thanks for the quick reviews!

@tobiasdiez tobiasdiez merged commit 862078a into master Apr 30, 2020
@tobiasdiez tobiasdiez deleted the fixFileFinder branch April 30, 2020 09:10
Siedlerchr added a commit that referenced this pull request Apr 30, 2020
…ionCaseInsensitive

* upstream/master:
  New Crowdin translations (#6375)
  Squashed 'src/main/resources/csl-styles/' changes from 143464e..906cd6d
  Fixes #6357: File directory (#6377)
  Disable the generate button if the ID field is empty (#6371)
  Fix Preferences style value too long (#6372)
  Fix various Dark theme issues (#6368)
  Correct label name in dependabot
  Bump java-diff-utils from 4.5 to 4.7 (#6365)
  Try with info.plist.template also (#6366)
  Fix wrong button order (Apply and Cancel) in ManageProtectedTermsDialog. (#6358)
  Bump flexmark-ext-gfm-strikethrough from 0.61.6 to 0.61.20 (#6361)
  Bump checkstyle from 8.31 to 8.32 (#6360)
  Bump flexmark-ext-gfm-tasklist from 0.61.16 to 0.61.20 (#6364)
  Bump flexmark from 0.61.16 to 0.61.20 (#6359)
  Bump org.beryx.jlink from 2.17.7 to 2.17.8 (#6362)
  Bump classgraph from 4.8.71 to 4.8.77 (#6363)
  Change blue and red colors in Merge entries dialog in Dark theme (#6356)
  Add Info plist to mac resources (#5986)
  Backward compatibility for 4.3.1 (#6296)
Siedlerchr added a commit that referenced this pull request May 2, 2020
* upstream/master: (166 commits)
  New Crowdin translations (#6382)
  Update code-howtos.md (#6393)
  Fix jstyle was invalid with default section at the start (#6386)
  Correcting file name for groups.uml (#6373)
  Fix underscore character being omitted from file name in Recent Libraries list (#6389)
  Rework journal abbreviation caching (#6304)
  Fix selecting custom export for copy to clipboard with uppercase file ext (#6290)
  New Crowdin translations (#6375)
  Squashed 'src/main/resources/csl-styles/' changes from 143464e..906cd6d
  Fixes #6357: File directory (#6377)
  Disable the generate button if the ID field is empty (#6371)
  Fix Preferences style value too long (#6372)
  Fix various Dark theme issues (#6368)
  Correct label name in dependabot
  Bump java-diff-utils from 4.5 to 4.7 (#6365)
  Try with info.plist.template also (#6366)
  Fix wrong button order (Apply and Cancel) in ManageProtectedTermsDialog. (#6358)
  Bump flexmark-ext-gfm-strikethrough from 0.61.6 to 0.61.20 (#6361)
  Bump checkstyle from 8.31 to 8.32 (#6360)
  Bump flexmark-ext-gfm-tasklist from 0.61.16 to 0.61.20 (#6364)
  ...
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.

3 participants