Copy to option #12374

priyanshu16095 · 2025-01-09T23:23:06Z

Fixes #12341

This PR introduces a 'Copy to' context menu option with features for cross-reference inclusion/exclusion, as well as the ability to save user preferences.

THE CHANGES MADE

Implemented a 'Copy to' context menu that displays the names of currently opened libraries as checkable menu items.
After selecting the libraries, a dialog is shown allowing users to include or exclude cross-references.
Added functionality to save user preferences for cross-reference inclusion/exclusion, ensuring a personalized experience for future use.

Mandatory checks

I own the copyright of the code submitted and I licence it under the MIT license
Change in CHANGELOG.md described in a way that is understandable for the average user (if change is visible to the user)
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.

…lusion and save user preference

github-actions

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

In case of issues with the import order, double check that you activated Auto Import.
You can trigger fixing imports by pressing Ctrl+Alt+O to trigger Optimize Imports.

github-actions

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

In case of issues with the import order, double check that you activated Auto Import.
You can trigger fixing imports by pressing Ctrl+Alt+O to trigger Optimize Imports.

… into copyToOption

github-actions

Your code currently does not meet JabRef's code guidelines.
We use Checkstyle to identify issues.
Please carefully follow the setup guide for the codestyle.
Afterwards, please run checkstyle locally and fix the issues.

In case of issues with the import order, double check that you activated Auto Import.
You can trigger fixing imports by pressing Ctrl+Alt+O to trigger Optimize Imports.

priyanshu16095 · 2025-01-11T19:37:48Z

Please review it and provide any suggestions.

Siedlerchr · 2025-01-11T20:30:28Z

We will do, please keep in mind that all maintainers work at jabref in their leisure time despite having a full time job. I will try to look into it tomorrow. Priyanshu Gupta ***@***.***> schrieb am Sa., 11. Jan. 2025, 20:38:

…

Please review it and provide any suggestions. — Reply to this email directly, view it on GitHub <#12374 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACOFZA3TI5CFOYLSVHGPHL2KFXKDAVCNFSM6AAAAABU5GXCISVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDKOBVGM4DMMJUGQ> . You are receiving this because you are subscribed to this thread.Message ID: ***@***.***>

priyanshu16095 · 2025-01-11T21:25:42Z

Apologies, and thanks for the update! I understand, and I really appreciate your time.

subhramit

Comments on first look.

subhramit · 2025-01-12T11:19:06Z

src/main/java/org/jabref/gui/edit/CopyTo.java

+    public void execute() {
+        Logger logger = LoggerFactory.getLogger(CopyTo.class);
+
+        BibDatabaseContext databaseContext = new BibDatabaseContext();


Not needed here.

Sure, I will remove them, considering it is a draft PR. I left them for explanatory purposes.

subhramit · 2025-01-12T11:20:15Z

src/main/java/org/jabref/gui/edit/CopyTo.java

+        for (String title: titles) {
+            logger.info("Selected Entreis: " + title);
+        }
+        for (String checkedPath: checkedPaths) {
+            logger.info("Selected Libraries to copy: " + checkedPath);
+        }


Remove (loops with no utility apart from logging)

Logic for copying the cross ref stuff is missing But I think @HoussemNasri splitted that task?

No, the copying of cross-ref stuff is the ticket assigned to @priyanshu16095.

#12339 is the ticket for creating the "copy to" menu; it is assigned to someone else, but since @priyanshu16095 already started working on it we agreed to have a draft pull request that might eventually be considered for merge in case of inactivity from the current assignee of that issue.

The issues/tasks are tightly coupled and meant to be done in a certain order. This pull request might or might not be merged depending on whether we receive any activities from the assignee of #12339, but IMO the work done here could be reused later to tackle #12341.

Logic for copying the cross ref stuff is missing But I think @HoussemNasri splitted that task?

Actually, #12340 was meant to be tackled before the one mentioned in PR description #12341, so yes logic for copy cross references is for another task or could be handled in this PR since both issues are assigned to @priyanshu16095.

subhramit · 2025-01-12T11:22:04Z

src/main/java/org/jabref/gui/edit/CopyTo.java

+        if (preferences.getCopyToPreferences().getShouldIncludeCrossReferences()) {
+            logger.info("Include Cross References");
+        } else {
+            logger.info("Exclude Cross References");
+        }


Either decrease log noise or remove.

subhramit · 2025-01-12T11:24:12Z

src/main/java/org/jabref/gui/edit/CopyTo.java

+                    optOut -> preferences.getCopyToPreferences().setShouldAskForIncludingCrossReferences(!optOut)
+            );
+        } else {
+            return preferences.getCopyToPreferences().getShouldIncludeCrossReferences();


See if you can improve abstraction - pass preferences.getCopyToPreferences() via constructor instead of entire GuiPreferences

subhramit · 2025-01-12T11:31:55Z

src/main/java/org/jabref/gui/maintable/RightClickMenu.java

+        if (!openDatabases.isEmpty()) {
+            openDatabases.forEach(library -> {
+                String path = library.getDatabasePath().get().toString();
+                String name = path.substring(path.lastIndexOf('\\') + 1);


@calixtus i think we can use FromAuxDialogViewModel#getDatabaseName() here?

Yeah we also use that code part in

jabref/src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java

Lines 122 to 130 in 7c958e7

Optional<String> dbOpt = Optional.empty();

if (database.getDatabasePath().isPresent()) {

dbOpt = FileUtil.getUniquePathFragment(stateManager.collectAllDatabasePaths(), database.getDatabasePath().get());

}

if (database.getLocation() == DatabaseLocation.SHARED) {

return database.getDBMSSynchronizer().getDBName() + " [" + Localization.lang("shared") + "]";

}

return dbOpt.orElseGet(() -> Localization.lang("untitled"));

We should add this somewhere more central

subhramit · 2025-01-12T11:34:25Z

Please also fix the Checkstyle, Openrewrite and Unit tests. The FAQ page will help you.

HoussemNasri · 2025-01-12T21:24:48Z

src/main/java/org/jabref/gui/edit/CopyToPreferences.java

+        this.shouldAskForIncludingCrossReferences.set(shouldAskForIncludingCrossReferences);
+    }
+
+    public boolean getShouldIncludeCrossReferences() {


How about shouldIncludeCrossReferences? It reads better.

Is it in line 17 or 14?

The method name. It is a nitpick so feel free to ignore.

HoussemNasri · 2025-01-12T21:30:20Z

src/main/java/org/jabref/gui/edit/CopyTo.java

+
+    @Override
+    public void execute() {
+        Logger logger = LoggerFactory.getLogger(CopyTo.class);


Each class should declare the logger only once. Refer to the logging usage in other classes for guidance.

Oh, I used it for temporary purposes, but forgot to remove it.

priyanshu16095 · 2025-01-14T08:52:37Z

I have fixed the Checkstyle, OpenRewrite, and Unit tests errors and made changes based on the reviews.

priyanshu16095 added 2 commits January 10, 2025 04:30

Implemented 'Copy to' context menu with cross-reference inclusion/exc…

507d51e

…lusion and save user preference

Merge remote-tracking branch 'upstream/main'

f502f68

priyanshu16095 marked this pull request as draft January 9, 2025 23:24

github-actions bot reviewed Jan 9, 2025

View reviewed changes

Merge branch 'main' into copyToOption

ba7536a

github-actions bot reviewed Jan 10, 2025

View reviewed changes

priyanshu16095 added 2 commits January 11, 2025 05:41

Fixing checkstye error

bb2b77c

Merge branch 'copyToOption' of https://github.com/priyanshu16095/jabref…

ca5775b

… into copyToOption

github-actions bot reviewed Jan 11, 2025

View reviewed changes

subhramit requested changes Jan 12, 2025

View reviewed changes

HoussemNasri reviewed Jan 12, 2025

View reviewed changes

priyanshu16095 added 2 commits January 13, 2025 04:51

Updated code based on feedback

31bb403

Use FileUtil to collect file paths

858015f

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Copy to option #12374

Copy to option #12374

priyanshu16095 commented Jan 9, 2025 •

edited

Loading

github-actions bot left a comment

github-actions bot left a comment

github-actions bot left a comment

priyanshu16095 commented Jan 11, 2025

Siedlerchr commented Jan 11, 2025 via email

priyanshu16095 commented Jan 11, 2025

subhramit left a comment

subhramit Jan 12, 2025

priyanshu16095 Jan 14, 2025

subhramit Jan 12, 2025

Siedlerchr Jan 12, 2025

HoussemNasri Jan 12, 2025

HoussemNasri Jan 12, 2025

subhramit Jan 12, 2025

subhramit Jan 12, 2025

subhramit Jan 12, 2025 •

edited

Loading

Siedlerchr Jan 12, 2025

subhramit commented Jan 12, 2025

HoussemNasri Jan 12, 2025

priyanshu16095 Jan 12, 2025

HoussemNasri Jan 12, 2025

HoussemNasri Jan 12, 2025

priyanshu16095 Jan 12, 2025 •

edited

Loading

priyanshu16095 commented Jan 14, 2025

	Optional<String> dbOpt = Optional.empty();
	if (database.getDatabasePath().isPresent()) {
	dbOpt = FileUtil.getUniquePathFragment(stateManager.collectAllDatabasePaths(), database.getDatabasePath().get());
	}
	if (database.getLocation() == DatabaseLocation.SHARED) {
	return database.getDBMSSynchronizer().getDBName() + " [" + Localization.lang("shared") + "]";
	}

	return dbOpt.orElseGet(() -> Localization.lang("untitled"));

Copy to option #12374

Are you sure you want to change the base?

Copy to option #12374

Conversation

priyanshu16095 commented Jan 9, 2025 • edited Loading

Fixes #12341

THE CHANGES MADE

Mandatory checks

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

github-actions bot left a comment

Choose a reason for hiding this comment

priyanshu16095 commented Jan 11, 2025

Siedlerchr commented Jan 11, 2025 via email

priyanshu16095 commented Jan 11, 2025

subhramit left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

subhramit Jan 12, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

subhramit commented Jan 12, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

priyanshu16095 Jan 12, 2025 • edited Loading

Choose a reason for hiding this comment

priyanshu16095 commented Jan 14, 2025

priyanshu16095 commented Jan 9, 2025 •

edited

Loading

subhramit Jan 12, 2025 •

edited

Loading

priyanshu16095 Jan 12, 2025 •

edited

Loading