Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Move files to subfolders in file directory #1899

Merged
merged 46 commits into from
Dec 7, 2016
Merged

Conversation

Siedlerchr
Copy link
Member

@Siedlerchr Siedlerchr commented Aug 31, 2016

Implementation for #1092

Already implemented Copy and Move methods from #1861

  • Change in CHANGELOG.md described
  • Tests created for changes
  • Screenshots added (for bigger UI changes)
  • Manually tested changed features in running JabRef
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)

jabreffiledirpattern

@Siedlerchr
Copy link
Member Author

TODO: Pass global as param
TODO: Check relative path

@Siedlerchr Siedlerchr changed the title [WIP] Create a new settings field for DirectoryPattern [WIP] Move files to subfolders in file directory Aug 31, 2016
Rename getFileDirectory to getFileDirectories
Create new method for returning the firstExistingFileDir
TODO: Add tests
TODO: Add targetDir in dlg
* upstream/master: (103 commits)
  add id fetcher in biblatexmode (#2006)
  Check available help languages when opening specific help page (#2005)
  Remove help suffix at help pages (#2004)
  French localization: Jabref_fr: empty strings translated + correction of a glitch (#2000)
  Disable gradle daemon - maybe, this brings back the integration tests
  French localization: Menu: update (#1999)
  Introduce convienent interface for ID-based fetcher (#1998)
  Updated dependencies (#1994)
  Some minor code cleanups (#1991)
  Add missing calls to showEntryEditor (which ensures that currentEditor is right)
  Remove obsolete adjustSplitter calls
  Remove FocusRequester as it is not executed threaded
  Rename variable "form" to "editor"
  use JabRefPreferences.getInstance() to fix ci
  simplify code
  always use fetcherexception's message for output
  Quickfix: Time limit for integration tests
  Fix Exception when Close Dialog with 'X'
  Enable tests at CircleCI build to ensure that only binary passing the basic tests are released.
  Fix japanese language files
  ...

# Conflicts:
#	src/main/java/net/sf/jabref/logic/cleanup/RenamePdfCleanup.java
Fix imports after merge
@Siedlerchr
Copy link
Member Author

@JabRef/developers I need some help with the GUI: I desperately tried to add the new field below the other, but I can't get it work, no matter what I tried 😡

The original layout code is this:
FormLayout layout = new FormLayout("1dlu, 8dlu, left:pref, 4dlu, fill:3dlu") in:
https://github.com/JabRef/jabref/blob/fileDirPattern/src/main/java/net/sf/jabref/gui/preftabs/ImportSettingsTab.java

The new field is there, but kinda hidden. No idea how to change it back:

settingsimportpanel

* upstream/master:
  Updated jabref_tr.properties (#2008)
Compatibily for old behaviour when no fileDirPattern is set
Use new prefs structure
* upstream/master: (102 commits)
  Removed unused test code (#2140)
  Fix main table bug when creating a duplicate (#2135)
  Remove explicit author and add SPDX-License-Identifier
  Remove GPL from README.md and CONTRIBUTING.md
  fix preview update (#2125)
  Remove some UnicodeToLatex uses (#2132)
  Fix mixup in french/farsi localization
  FetcherException should extend JabRefException
  Fix exception when opening preference dialog (#2127)
  Unify ParserException and ParseException (#2124)
  Small refactoring in Importer package (#2053)
  Implement Datepicker "none"-button (#2122)
  revert change from 816d30c
  Change title/tooltip of source panel in biblatex mode (#2120)
  Refactoring: completey typed metadata and add detailed travis output (#2112)
  RTFchars fix (#2097)
  Fix NPE in Medline fetcher on missing ISSN (#2113)
  Ctrl-s parsing error message (#2114)
  Fix bad web search error messages (#2034)
  parse error freeze (#2106)
  ...

# Conflicts:
#	src/main/java/net/sf/jabref/collab/FileUpdateMonitor.java
#	src/main/java/net/sf/jabref/gui/externalfiles/DownloadExternalFile.java
#	src/main/java/net/sf/jabref/gui/externalfiles/DroppedFileHandler.java
#	src/main/java/net/sf/jabref/gui/externalfiles/MoveFileAction.java
#	src/main/java/net/sf/jabref/logic/cleanup/RenamePdfCleanup.java
#	src/main/java/net/sf/jabref/logic/exporter/FileSaveSession.java
#	src/main/java/net/sf/jabref/logic/util/io/FileUtil.java
#	src/main/java/net/sf/jabref/preferences/JabRefPreferences.java
@Siedlerchr
Copy link
Member Author

Yeah, finally the layout works 😎

* upstream/master: (24 commits)
  hotfix NPE in the main table (#2158)
  Enhance side pane toggle (#1605)
  Added gray background text to authors field to assist newcomers (#2147)
  Rewrite google scholar fetcher to new infrastructure (#2101)
  Found entries will be shown when dropping a pdf with xmp meta data (#2150)
  Japanese translation (#2155)
  Add shortcuts to context menu (#2131)
  [WIP] Doi resolution ignore (#2154)
  Fix DuplicationChecker and key generator (#2151)
  just close popup on first ESC
  keep entry in sight
  Fix isSharedDatabaseAlreadyPresent check.
  don't change entry after search if editing an entry (#2129)
  Change download URL to downloads.jabref.org (#2145)
  fix switching edited textfield in the entry editor with TAB (#2138)
  Update me.champeau.gradle.jmh' version from 0.3.0 to 0.3.1
  Update install4j plugin from 6.1.2 to 6.1.3
  Remove gradle plugin com.github.youribonnaffe.gradle.format as it is deprecated and the config uses a non-existing file
  Update gradle from 3.0 to 3.1
  Fix changing the font size not working when importing preferences (#2069)
  ...

# Conflicts:
#	src/main/java/net/sf/jabref/gui/externalfiles/DroppedFileHandler.java
#	src/main/java/net/sf/jabref/gui/importer/fetcher/GoogleScholarFetcher.java
* upstream/master:
  check whether test is executed at circleCI
Use default setting of Empty string
fix layout
@dierkes
Copy link

dierkes commented Oct 13, 2016

Thanks for the work! I'm really missing this feature in JabRef 3.

However, the current behavior is a little unexpected. First, the file seems to be moved relative to its current directory. For example directory pattern '\EntryType' moves the file with every execution of cleanup to a new subdirectory. For me, expected behavior would be, that the new path is relative to the directory of the bib-file or some otherwise specified root directory.

Second, the file field in the bib entry seems to be adjusted only, if the option 'Use the BIB file location as primary file directory' is set. Otherwise, the file is moved, but the file path is not adjusted.

@Siedlerchr
Copy link
Member Author

@dierkes Thanks for your feedback. I would have asked you to test it otherwise, too ;)
The moving files is a bit complicated as it has to be kept backwards compatible.

The file directory is the first found directory: a) global file dire from setting b) bib file relative c) bib specific setting (atm not sure about the order b and c)

Regarding the file field, I will check it

@dierkes
Copy link

dierkes commented Oct 14, 2016

Regarding the new file directory: That's not the behavior I observed.

My test case was:
original file: phdthesis/xyz_2015.pdf (all paths relative to the bib-file)
file name pattern: '\bibtexkey'
file dir pattern: '\EntryType'

cleanup -> phdthesis/PhdThesis/xyz_215.pdf
cleanup -> phdthesis/PhdThesis/PhdThesis/xyz_215.pdf
cleanup -> phdthesis/PhdThesis/PhdThesis/PhdThesis/xyz_215.pdf

It didn't matter if 'Use the BIB file location as primary file directory' was set or if the default directory for files was set. (I tried with setting it to the directory of the bib-file.) I didn't try to set the default file directory for the specific bib-file, however. (Can do that later.)

From my perspective, a backward compatible solution could be: If the file dir pattern is not set, rename the file in its current location. If it is set, move the file to the directory specified by file dir pattern (relative to bib-file directory, if it's a relative path) and rename it there. However, an empty file name pattern could mean a) rename the file in place or b) move file to bib-file directory, so an additional option is probably necessary, that specifies whether moving files is active or not.

Interpreting the file dir pattern as a path relative to the current directory of the file doesn't make sense to me, since executing cleanup would always change the file path. Which is probably the opposite effect one expects from a cleanup ;)

@Siedlerchr
Copy link
Member Author

Ah I see you are right.

Will look around.

2016-10-14 12:48 GMT+02:00 dierkes notifications@github.com:

Regarding the new file directory: That's not the behavior I observed.

My test case was:
original file: phdthesis/xyz_2015.pdf (all paths relative to the bib-file)
file name pattern: '\bibtexkey'
file dir pattern: '\EntryType'

cleanup -> phdthesis/PhdThesis/xyz_215.pdf
cleanup -> phdthesis/PhdThesis/PhdThesis/xyz_215.pdf
cleanup -> phdthesis/PhdThesis/PhdThesis/PhdThesis/xyz_215.pdf

It didn't matter if 'Use the BIB file location as primary file directory'
was set or if the default directory for files was set. (I tried with
setting it to the directory of the bib-file.) I didn't try to set the
default file directory for the specific bib-file, however. (Can do that
later.)

From my perspective, a backward compatible solution could be: If the file
dir pattern is not set, rename the file in its current location. If it is
set, move the file to the directory specified by file dir pattern (relative
to bib-file directory, if it's a relative path) and rename it there.
However, an empty file name pattern could mean a) rename the file in place
or b) move file to bib-file directory, so an additional option is probably
necessary, that specifies whether moving files is active or not.

Interpreting the file dir pattern as a path relative to the current
directory of the file doesn't make sense to me, since executing cleanup
would always change the file path. Which is probably the opposite effect
one expects from a cleanup ;)


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#1899 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AATi5PlBiKVFhsla2g3oROQ4oKL60iRdks5qz135gaJpZM4Jxp95
.

* upstream/master:
  Code cleanups (#2141)
  Remove obsolete string in .mailmap
  Update .mailmap and AUTHORS
  Implementation of autosave & backup feature (#2118)
  Make all files selectable in file chooser dialogs (#2094)
  Fix group drag and drop (#2161)
  Change donation badge and point to https://donations.jabref.org

# Conflicts:
#	src/main/java/net/sf/jabref/logic/util/OptionalUtil.java
@Siedlerchr
Copy link
Member Author

@dierkes I fixed it, when all tests are green you could try it out.

@dierkes
Copy link

dierkes commented Oct 15, 2016

Now it works mostly as expected for me.

Of course, as described above, with the current state it's not possible to rename a file only (if it's current location doesn't match the file dir pattern). So we might need the option to activate/deactivate moving the files.

And a very strange thing I noticed: If 'FileDir pattern' is empty and 'Main file directory' is empty and 'Use the BIB file location as primary file directory' is not set, then the file is not moved and a directory relative to the JabRef directory (or maybe to the current working directory) is created with the name of the desired file path. Staying in the example from above, instead of renaming the file to
(literature/) PhdThesis/xyz_2015.pdf
a directory named
(jabref/) PhdThesis/xyz_2015.pdf/
is created.

@Siedlerchr
Copy link
Member Author

Thanks for the feedback, I will check it
Normally renaming file only should work if fileDirPattern is empty in the
settings

Am 15.10.2016 11:52 nachm. schrieb "dierkes" notifications@github.com:

Now it works mostly as expected for me.

Of course, as described above, with the current state it's not possible to
rename a file only (if it's current location doesn't match the file dir
pattern). So we might need the option to activate/deactivate moving the
files.

And a very strange thing I noticed: If 'FileDir pattern' is empty and
'Main file directory' is empty and 'Use the BIB file location as primary
file directory' is not set, then the file is not moved and a directory
relative to the JabRef directory (or maybe to the current working
directory) is created with the name of the desired file path. Staying in
the example from above, instead of renaming the file to
(literature/) PhdThesis/xyz_2015.pdf
a directory named
(jabref/) PhdThesis/xyz_2015.pdf/
is created.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#1899 (comment), or mute
the thread
https://github.com/notifications/unsubscribe-auth/AATi5AJdZEgO8bkoYc7N76tm89c8-MvIks5q0UshgaJpZM4Jxp95
.

@Siedlerchr
Copy link
Member Author

@dierkes I fixed the strange behaviour. There was some directory path which was just "" and not null, so it was resolved to the current dir of the jar. Now it only renames the file.
And the old behaviour now works, too, when fileDirPattern is Empty, the file is moved to the file directory,
The file directories are checked in this order:
The settings are prioritized in the following order and the first defined setting is used

  1. metadata user-specific directory
  2. metadata general directory
  3. preferences directory
  4. BIB file directory

@Siedlerchr
Copy link
Member Author

included feedbacjk

* upstream/master:
  builds.jabref.org: Keep html files and just remove *.exe, *.dmg, *.jar during upload of a new build
  Fix comment typo
  Fix typos in comments in .travis.yml
  Switch to LGoodDatePicker (#2340)
  Gradle build scans should only run on TravisCI, not on the other CI services
  Try scans.gradle.com
  Remove reference to GPL - we are MIT now
  fix date changes in medline fetcher results
  Fix #2336: shutdown is working again (#2338)

# Conflicts:
#	src/main/java/net/sf/jabref/model/database/BibDatabaseContext.java
@stefan-kolb
Copy link
Member

One or two of my comments are still open when I look at the Github UI above. Actually I'm only interested why you initialized the Array with a length of 4, I'm fine with the rest as thes are only cosmetic changes.

@Siedlerchr
Copy link
Member Author

Fixed the rest and the one test which @koppor commented out, in c9726d9 works fine on travis now again.

@stefan-kolb
Copy link
Member

@JabRef/developers we can merge right?

@lenhard
Copy link
Member

lenhard commented Dec 6, 2016

@stefan-kolb @Siedlerchr Yes we can. That is, once the merge conflict in the italian localization gets fixed.

* upstream/master:
  Translate new messages (#2341)
  Update gradle from 3.2 to 3.2.1
  Update com.github.johnrengelman.shadow from 1.2.3 to 1.2.4
  Introduce categories: DatabseTests, FetcherTests, GUITests
  Place oracle stub into src/main/java
  Move all tests into src/test/java

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
* upstream/master:
  Translate new messages (#2341)
  Update gradle from 3.2 to 3.2.1
  Update com.github.johnrengelman.shadow from 1.2.3 to 1.2.4
  Introduce categories: DatabseTests, FetcherTests, GUITests
  Place oracle stub into src/main/java
  Move all tests into src/test/java

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
…o fileDirPattern

* 'fileDirPattern' of https://github.com/JabRef/jabref:
  Merge remote-tracking branch 'upstream/master' into fileDirPattern

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
* upstream/master:
  Translate new messages (#2341)
  Update gradle from 3.2 to 3.2.1
  Update com.github.johnrengelman.shadow from 1.2.3 to 1.2.4
  Introduce categories: DatabseTests, FetcherTests, GUITests
  Place oracle stub into src/main/java
  Move all tests into src/test/java
@Siedlerchr
Copy link
Member Author

@JabRef/developers all Conflicts fixed.

@dierkes
Copy link

dierkes commented Dec 6, 2016

Hi, on my system two tests are failing, cleanupRenamePdfRenamesWithMultipleFiles and cleanUpRenamePdfRenameFileDirectoryPatternSubDirectory. However, the problem seems to be that these and other tests do not remove the temporary files properly after execution. The behavior of the cleanup function itself is as expected, but since a file with the target path exists already (left from another test), the attached file is not renamed and the assertion fails. I'm running the tests on Fedora.

@Siedlerchr
Copy link
Member Author

Siedlerchr commented Dec 7, 2016 via email

@Siedlerchr
Copy link
Member Author

Should now be fixed. Was some Preferences problem as I already thought. Mocked it away.

@stefan-kolb stefan-kolb merged commit 5086e83 into master Dec 7, 2016
@stefan-kolb stefan-kolb deleted the fileDirPattern branch December 7, 2016 20:02
Siedlerchr added a commit that referenced this pull request Dec 7, 2016
* upstream/master:
  Move files to subfolders in file directory (#1899)

# Conflicts:
#	src/test/java/net/sf/jabref/logic/cleanup/CleanupWorkerTest.java
tobiasdiez pushed a commit to tobiasdiez/jabref that referenced this pull request Dec 10, 2016
* Create a new settings field for DirectoryPattern
ATM hardcoded to entrytype
Drag and Drop file now respects fileDirPattern

* Cleanup/Rename PDF now moves the file to the targetDir, too

* Cleanup operation now works, too
Rename getFileDirectory to getFileDirectories
Create new method for returning the firstExistingFileDir
TODO: Add tests
TODO: Add targetDir in dlg

* Change layout back to default
Fix imports after merge

* Add fileDirPattern to Prefs
Compatibily for old behaviour when no fileDirPattern is set
Use new prefs structure

* Fix NPE by settting BibLocation as primary Dir

* TearDown to @after

* Fix merge conflicts

* Fix failing tests
Use default setting of Empty string
fix layout

* Fix moving/creating directories files on every cleanup

* Fix: When fileDirPattern is Empty only rename the file (backwards compatibility)

* Fix Rename file only if no fileDirPattern is present in settings

* Moved return out of for-loop

* Reactivated test

* Fix failing test, set ImportFileDirPattern default to empty

* Fix whitespace

* Add some junit tests for fileDirPatterns

* Add localization, fix checkstyle

* Make junit test methods OS independent
Remove debug outputs

* Added changelog entry, fix lang error

* Fix empty lines and remove Sysouts
Reformat

* Remove settings prefs und fixed bug

* Fix empty lines and duplicate lang key

* Remove assumption to test ci build again

* Remove intialization, fix empty lines

* Merge remote-tracking branch 'upstream/master' into fileDirPattern

* upstream/master:
  Translate new messages (JabRef#2341)
  Update gradle from 3.2 to 3.2.1
  Update com.github.johnrengelman.shadow from 1.2.3 to 1.2.4
  Introduce categories: DatabseTests, FetcherTests, GUITests
  Place oracle stub into src/main/java
  Move all tests into src/test/java

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.

* Merge remote-tracking branch 'upstream/master' into fileDirPattern

* upstream/master:
  Translate new messages (JabRef#2341)
  Update gradle from 3.2 to 3.2.1
  Update com.github.johnrengelman.shadow from 1.2.3 to 1.2.4
  Introduce categories: DatabseTests, FetcherTests, GUITests
  Place oracle stub into src/main/java
  Move all tests into src/test/java

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.

* Fix conflict

* Merge branch 'fileDirPattern' of https://github.com/JabRef/jabref into fileDirPattern

* 'fileDirPattern' of https://github.com/JabRef/jabref:
  Merge remote-tracking branch 'upstream/master' into fileDirPattern

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.

* Fix conflict

* Mock FileDirectoryPreferences to fix cleanup problems
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
[outdated] type: enhancement 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.

7 participants