Enable groups drag'n'drop to new library #9460

rafatd · 2022-12-16T04:22:36Z

Change in CHANGELOG.md described in a way that is understandable for the average user (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 developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

ThiloteE · 2022-12-16T07:57:59Z

To ease organizational workflows I have linked the pull-request to the issue with syntax as described in https://docs.github.com/en/issues/tracking-your-work-with-issues/linking-a-pull-request-to-an-issue

Linking a pull request to an issue using a keyword

You can link a pull request to an issue by using a supported keyword in the pull request's description or in a commit message. The pull request must be on the default branch.

close

closes

closed

fix

fixes

fixed

resolve

resolves

resolved

If you use a keyword to reference a pull request comment in another pull request, the pull requests will be linked. Merging the referencing pull request also closes the referenced pull request.

The syntax for closing keywords depends on whether the issue is in the same repository as the pull request.

Siedlerchr · 2022-12-18T22:51:58Z

Please have a look at the checkstlye/reviewdog tests

fixing all issues raised by checkstlye/reviewdog tests

rafatd · 2022-12-19T00:45:24Z

Please have a look at the checkstlye/reviewdog tests

I have tried many times to fix these issues, but still I keep getting this error "Use a single space to separate non-whitespace characters." Can anyone please help me understand what that mean?

Siedlerchr · 2022-12-19T17:43:34Z

@rafatd the best is to setup JabRef's code style https://devdocs.jabref.org/getting-into-the-code/guidelines-for-setting-up-a-local-workspace.html#using-jabrefs-code-style
Then you can just use the ide formatting

PS: I don't get the line as welll

src/main/java/org/jabref/gui/JabRefFrame.java

Siedlerchr · 2022-12-19T17:52:01Z

src/main/java/org/jabref/gui/JabRefFrame.java

-                                    ((LibraryTab) libraryTab).dropEntry(stateManager.getLocalDragboard().getBibEntries());
+                                    if (dragboard.hasContent(DragAndDropDataFormats.GROUP)) {
+                                        List<String> groupPathToSources = (List<String>) dragboard.getContent(DragAndDropDataFormats.GROUP);
+                                        if (((LibraryTab) libraryTab).getBibDatabaseContext().getMetaData().getGroups().isEmpty()) {


I would extract this casting to LibraryTab to a new variable, then you don't have to add the casting everywhere
e.g. LibraryTab libraryTab = (LibraryTab) tab;

Noted, thanks for that, I will update the file accordingly. Thanks.

I can confirm that this has been updated.

Siedlerchr · 2022-12-19T17:53:58Z

Tested your feature, looks good so far. Just one thing I noticed: Colors and icons are not transfered. Don't know if this is possible as well

Extract this casting to LibraryTab to a new variable. Fix the "Use a single space to separate non-whitespace characters." error.

rafatd · 2022-12-19T18:12:58Z

Tested your feature, looks good so far. Just one thing I noticed: Colors and icons are not transferred. Don't know if this is possible as well

I just noticed that. That's weird, since the colour and the icon are stored in the abstract group object, and I am deeply copying that. So I am confused as to why it's not transferring them. I will try to make more research regarding that. In the main time, I can confirm that my last commit passes all the tests. Let me know if I must change anything, or if you are happy with that.

Enable transferring colours and icons of each group.

rafatd · 2022-12-19T18:27:32Z

Tested your feature, looks good so far. Just one thing I noticed: Colors and icons are not transfered. Don't know if this is possible as well

Hi again, I can confirm that I fixed that, tuners the deepcopy() function only copy deep copying an explicitgroup object, which does not know anything about the colours and the icons. So instead I choose to copy the whole GroupTreeNode.
Let me know if that works on your end.

ThiloteE · 2022-12-19T20:44:17Z

I tried the pull-request with latest commits just now and found that groups were correctly copied (as far as I could see), but encountered issues related to:

duplicate detection
- have multiple entries (maybe even similar entries) in the groups and drag n' drop to new library --> entry merge editor opens. Entry merge editor should probably not open, since we simply want to copy. (Although it helps to find bugs, when the entry editor opens :D)

rafatd · 2022-12-19T20:52:09Z

I tried the pull-request with latest commits just now and found that groups were correctly copied (as far as I could see), but encountered issues related to:

duplicate detection

have multiple entries (maybe even similar entries) in the groups and drag n' drop to new library --> entry merge editor opens. Entry merge editor should probably not open, since we simply want to copy. (Although it helps to find bugs, when the entry editor opens :D)

I believe that I cannot reproduce this error on my local machine. If you have the same entires the merge editor will be open, this had nothing to do with my feature I am assuming. It would be helpful if you can send me the exact steps you did to get that?

…of_groups_to_new_library * upstream/main: (148 commits) Fix remember last open valid library with empty new one (JabRef#9489) Observable Preferences R (CitationKeyPatternPreferences) (JabRef#9486) Fixed ZBMathTest and extracted keyWordSeparator (JabRef#9485) New Crowdin updates (JabRef#9483) Add log for ignored excepton (JabRef#9302) Select Library to import into (JabRef#9402) Bump org.eclipse.jgit from 6.3.0.202209071007-r to 6.4.0.202211300538-r (JabRef#9476) Bump com.github.andygoossens.modernizer from 1.6.2 to 1.7.0 (JabRef#9478) Bump mockito-core from 4.9.0 to 4.10.0 (JabRef#9479) Bump checkstyle from 10.4 to 10.5.0 (JabRef#9477) Bump slf4j-api from 2.0.5 to 2.0.6 in /buildSrc (JabRef#9480) Bibtex month not deprecated (JabRef#9404) Show development information\n\n+semver: major Release v5.8 Update external libraries add afterburner fx jabref add jakarta inject fix display name for artifact store Prepare CHANGELOG for release Also trigger on branch arm64mac-release Only arm64 m+ Update deployment-arm64.yml ...

HoussemNasri · 2022-12-20T23:08:19Z

After dragging and dropping a group and then deleting it, a notification that says "The library has been modified by another program" is triggered. However, it seems to be an old bug that was not introduced in this PR. #9064

rafatd · 2022-12-20T23:16:37Z

After dragging and dropping a group and then deleting it, a notification that says "The library has been modified by another program" is triggered. However, it seems to be an old bug that was not introduced in this PR. #9064

I cannot reproduce this issue, as you say it's an old bug! does fixing the issue JabRef#530 also require fixing that?

HoussemNasri · 2022-12-21T08:36:58Z

You can ignore the comment. It's for documentation purposes to clarify that the bug existed before this PR. It's definitely not a requirement to fix the bug, but you're welcome to give it a try. However, keep in mind that it is not an easy catch. We had many previous attempts of fixing it but it keeps coming back.

rafatd · 2022-12-21T08:48:17Z

You can ignore the comment. It's for documentation purposes to clarify that the bug existed before this PR. It's definitely not a requirement to fix the bug, but you're welcome to give it a try. However, keep in mind that it is not an easy catch. We had many previous attempts of fixing it but it keeps coming back.

Great, I am not sure that I have enough time to be working on that.
Is there anything else you reckon needs to be changed?

HoussemNasri

I tried out the feature on my local machine and it works great! However, I have a few comments about the code.

src/main/java/org/jabref/gui/JabRefFrame.java

rafatd · 2022-12-21T14:42:22Z

Hi @HoussemNasri,

Thanks for your feedback, I have done some updates to the files. I hope that now they should be ok now. I added some methods to DragAndDropHelper class. I changed the names of several variables as you requested. I stopped passing allEntries.

With regards to the nested if statement, I have tried my best to reduce that, and as far as I know, I do not think there is a better way to reduce it more.

Let me know what you think.

src/main/java/org/jabref/gui/JabRefFrame.java

…of_groups_to_new_library * upstream/main: Update MacOS jabrefHost.py to find local installs (JabRef#9487)

* upstream/main: (145 commits) Enable groups drag'n'drop to new library (JabRef#9460) Update MacOS jabrefHost.py to find local installs (JabRef#9487) Fix remember last open valid library with empty new one (JabRef#9489) Observable Preferences R (CitationKeyPatternPreferences) (JabRef#9486) Fixed ZBMathTest and extracted keyWordSeparator (JabRef#9485) New Crowdin updates (JabRef#9483) Add log for ignored excepton (JabRef#9302) Select Library to import into (JabRef#9402) Bump org.eclipse.jgit from 6.3.0.202209071007-r to 6.4.0.202211300538-r (JabRef#9476) Bump com.github.andygoossens.modernizer from 1.6.2 to 1.7.0 (JabRef#9478) Bump mockito-core from 4.9.0 to 4.10.0 (JabRef#9479) Bump checkstyle from 10.4 to 10.5.0 (JabRef#9477) Bump slf4j-api from 2.0.5 to 2.0.6 in /buildSrc (JabRef#9480) Bibtex month not deprecated (JabRef#9404) Show development information\n\n+semver: major Release v5.8 Update external libraries add afterburner fx jabref add jakarta inject fix display name for artifact store Prepare CHANGELOG for release Also trigger on branch arm64mac-release ... # Conflicts: # CHANGELOG.md # src/main/java/org/jabref/preferences/JabRefPreferences.java

* upstream/main: (120 commits) Enable groups drag'n'drop to new library (JabRef#9460) Update MacOS jabrefHost.py to find local installs (JabRef#9487) Fix remember last open valid library with empty new one (JabRef#9489) Observable Preferences R (CitationKeyPatternPreferences) (JabRef#9486) Fixed ZBMathTest and extracted keyWordSeparator (JabRef#9485) New Crowdin updates (JabRef#9483) Add log for ignored excepton (JabRef#9302) Select Library to import into (JabRef#9402) Bump org.eclipse.jgit from 6.3.0.202209071007-r to 6.4.0.202211300538-r (JabRef#9476) Bump com.github.andygoossens.modernizer from 1.6.2 to 1.7.0 (JabRef#9478) Bump mockito-core from 4.9.0 to 4.10.0 (JabRef#9479) Bump checkstyle from 10.4 to 10.5.0 (JabRef#9477) Bump slf4j-api from 2.0.5 to 2.0.6 in /buildSrc (JabRef#9480) Bibtex month not deprecated (JabRef#9404) Show development information\n\n+semver: major Release v5.8 Update external libraries add afterburner fx jabref add jakarta inject fix display name for artifact store Prepare CHANGELOG for release Also trigger on branch arm64mac-release ...

* upstream/main: (75 commits) Observable Preferences S (LastExportPath and Cleanups in JabRefPreferences and Globals) (#9493) Enable groups drag'n'drop to new library (#9460) Update MacOS jabrefHost.py to find local installs (#9487) Fix remember last open valid library with empty new one (#9489) Observable Preferences R (CitationKeyPatternPreferences) (#9486) Fixed ZBMathTest and extracted keyWordSeparator (#9485) New Crowdin updates (#9483) Add log for ignored excepton (#9302) Select Library to import into (#9402) Bump org.eclipse.jgit from 6.3.0.202209071007-r to 6.4.0.202211300538-r (#9476) Bump com.github.andygoossens.modernizer from 1.6.2 to 1.7.0 (#9478) Bump mockito-core from 4.9.0 to 4.10.0 (#9479) Bump checkstyle from 10.4 to 10.5.0 (#9477) Bump slf4j-api from 2.0.5 to 2.0.6 in /buildSrc (#9480) Bibtex month not deprecated (#9404) Show development information\n\n+semver: major Release v5.8 Update external libraries add afterburner fx jabref add jakarta inject fix display name for artifact store Prepare CHANGELOG for release ...

groups drag'n'drop enabled

2e9bbd3

rafatd mentioned this pull request Dec 16, 2022

Enable drag'n'drop of groups to new library JabRef/jabref-koppor#530

Closed

ThiloteE added the groups label Dec 16, 2022

rafatd added 8 commits December 18, 2022 23:17

Update JabRefFrame.java

dea02f8

Update JabRefFrame.java

60d06cf

fixing all issues raised by checkstlye/reviewdog tests

Update JabRefFrame.java

4c85d9a

Update JabRefFrame.java

7d75e7e

Update JabRefFrame.java

23bdde2

Update JabRefFrame.java

a435a7f

Update JabRefFrame.java

c55d74a

Update JabRefFrame.java

6581b61

Update JabRefFrame.java

c33fe4c

Siedlerchr changed the title ~~groups drag'n'drop enabled~~ Enable groups drag'n'drop to new library Dec 19, 2022

Siedlerchr reviewed Dec 19, 2022

View reviewed changes

src/main/java/org/jabref/gui/JabRefFrame.java Show resolved Hide resolved

Siedlerchr reviewed Dec 19, 2022

View reviewed changes

Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Dec 19, 2022

Update JabRefFrame.java

ed94f2c

Extract this casting to LibraryTab to a new variable. Fix the "Use a single space to separate non-whitespace characters." error.

Update JabRefFrame.java

591f2c3

Enable transferring colours and icons of each group.

Siedlerchr requested a review from HoussemNasri December 20, 2022 22:12

HoussemNasri requested changes Dec 21, 2022

View reviewed changes

rafatd and others added 4 commits December 21, 2022 14:19

fix issues raised by HoussemNasri

86105d5

fix issues raised by HoussemNasri

7281f0b

fix conflict in JabRefFrame.java

a9e0980

Update JabRefFrame.java

04ece57

Siedlerchr reviewed Dec 21, 2022

View reviewed changes

src/main/java/org/jabref/gui/JabRefFrame.java Outdated Show resolved Hide resolved

Siedlerchr added 3 commits December 21, 2022 17:17

Merge remote-tracking branch 'upstream/main' into enable_drag'n'drop_…

ca9197c

…of_groups_to_new_library * upstream/main: Update MacOS jabrefHost.py to find local installs (JabRef#9487)

remove checkstyle

8be09f7

fix merge conflicts

c22b8df

Siedlerchr previously approved these changes Dec 21, 2022

View reviewed changes

fix checkstyle

2ce1ea4

Siedlerchr dismissed their stale review via 2ce1ea4 December 21, 2022 16:41

Siedlerchr merged commit 41c8b48 into JabRef:main Dec 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enable groups drag'n'drop to new library #9460

Enable groups drag'n'drop to new library #9460

rafatd commented Dec 16, 2022 •

edited

Loading

ThiloteE commented Dec 16, 2022

Linking a pull request to an issue using a keyword

Siedlerchr commented Dec 18, 2022

rafatd commented Dec 19, 2022

Siedlerchr commented Dec 19, 2022 •

edited

Loading

Siedlerchr Dec 19, 2022

rafatd Dec 19, 2022

rafatd Dec 19, 2022

Siedlerchr commented Dec 19, 2022

rafatd commented Dec 19, 2022

rafatd commented Dec 19, 2022

ThiloteE commented Dec 19, 2022

rafatd commented Dec 19, 2022

HoussemNasri commented Dec 20, 2022

rafatd commented Dec 20, 2022

HoussemNasri commented Dec 21, 2022

rafatd commented Dec 21, 2022

HoussemNasri left a comment

rafatd commented Dec 21, 2022

Enable groups drag'n'drop to new library #9460

Enable groups drag'n'drop to new library #9460

Conversation

rafatd commented Dec 16, 2022 • edited Loading

ThiloteE commented Dec 16, 2022

Linking a pull request to an issue using a keyword

Siedlerchr commented Dec 18, 2022

rafatd commented Dec 19, 2022

Siedlerchr commented Dec 19, 2022 • edited Loading

Siedlerchr Dec 19, 2022

Choose a reason for hiding this comment

rafatd Dec 19, 2022

Choose a reason for hiding this comment

rafatd Dec 19, 2022

Choose a reason for hiding this comment

Siedlerchr commented Dec 19, 2022

rafatd commented Dec 19, 2022

rafatd commented Dec 19, 2022

ThiloteE commented Dec 19, 2022

rafatd commented Dec 19, 2022

HoussemNasri commented Dec 20, 2022

rafatd commented Dec 20, 2022

HoussemNasri commented Dec 21, 2022

rafatd commented Dec 21, 2022

HoussemNasri left a comment

Choose a reason for hiding this comment

rafatd commented Dec 21, 2022

rafatd commented Dec 16, 2022 •

edited

Loading

Siedlerchr commented Dec 19, 2022 •

edited

Loading