Add a "view as BibTeX" option before importing an entry from the cita… #11847

JamseBonde007 · 2024-09-29T08:34:13Z

Add a "view as BibTeX" option before importing an entry from the cita…tion relation tab #11826

This PR implements the feature requested in issue #10869. It adds a button that allows users to display a dialog showing the full BibTeX entry when importing citations from the Citation Relation tab. This enhancement will help users decide whether to import a potentially "duplicate" record by allowing them to see the full details of the entry in BibTeX format.

Closes #11826

Mandatory checks

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.

…tion relation tab JabRef#11826

koppor · 2024-09-29T11:58:05Z

src/main/java/org/jabref/gui/entryeditor/SourceTab.java

+            ca.appendText(getSourceString(entry, mode, fieldPreferences));
+        } catch (
+                IOException ex) {
+            ca = null;


Never return null. Use Optional<CodeArea> or throw an exception: The caller should explicitly know that null is returned.

Reason: We do not rely on null annotations really.

koppor · 2024-09-29T11:58:25Z

src/main/java/org/jabref/gui/entryeditor/SourceTab.java

+        } catch (
+                IOException ex) {


Try to keep it in one line. We know that IntelliJ outformats it like that - we did not find a way to fix this issue.

koppor · 2024-09-29T12:10:39Z

src/main/resources/l10n/JabRef_en.properties

@@ -2782,3 +2782,8 @@ Warning\:\ The\ selected\ directory\ is\ not\ a\ valid\ directory.=Warning: The
 Currently\ selected\ JStyle\:\ '%0' = Currently selected JStyle: '%0'
 Currently\ selected\ CSL\ Style\:\ '%0' = Currently selected CSL Style: '%0'
 Store\ url\ for\ downloaded\ file=Store url for downloaded file
+
+BibTeX\ of\ that\ entry=BibTeX of that entry


Re-use "Show BibTeX source" (or "Show %0 source". Just check the existing strings.)

Exits only "Show/edit %0 source"

koppor · 2024-09-29T12:10:57Z

src/main/resources/l10n/JabRef_en.properties

+
+BibTeX\ of\ that\ entry=BibTeX of that entry
+Could\ not\ load\ %0\ source=Could not load %0 source
+BibTeX\ source=BibTeX source


Please check the existing one. I think, it is %0 source.

koppor · 2024-09-29T12:11:54Z

src/main/resources/l10n/JabRef_en.properties

@@ -2782,3 +2782,8 @@ Warning\:\ The\ selected\ directory\ is\ not\ a\ valid\ directory.=Warning: The
 Currently\ selected\ JStyle\:\ '%0' = Currently selected JStyle: '%0'
 Currently\ selected\ CSL\ Style\:\ '%0' = Currently selected CSL Style: '%0'
 Store\ url\ for\ downloaded\ file=Store url for downloaded file
+
+BibTeX\ of\ that\ entry=BibTeX of that entry
+Could\ not\ load\ %0\ source=Could not load %0 source


Not more information than "Error". Thus, please use "Error" as string - and not a new text.

koppor · 2024-09-29T12:13:25Z

src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java

+    private EntryEditor createEntryEditor() {
+        Supplier<LibraryTab> tabSupplier = () -> this.libraryTab;
+        return new EntryEditor(this.libraryTab,
+                // Actions are recreated here since this avoids passing more parameters and the amount of additional memory consumption is neglegtable.


I think, the main reason is that undo and redo are independent here. Thus, these should be fake ones? -- The BibTeX is from the external service and NOT contained in the current library. Thus, it has NOTHING to do with the current library and thus no undo/redo relation with it.

Update: I think, this is not necessary at all. No entry editor is needed.

koppor · 2024-09-29T12:14:03Z

src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java

+        if (codeArea == null) {
+            dialogService.showWarningDialogAndWait(Localization.lang("BibTeX source", databaseContext.getMode().getFormattedName()), Localization.lang("Could not load %0 source", databaseContext.getMode().getFormattedName()));


How often does this occur? Bug in the code or real use? I think, its bug in the code. Thus, please use LOGGER.warn(...) instead of a dialog service.

Also just return then. #FailFast. Do not use } else { .

koppor · 2024-09-29T12:18:57Z

src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java

+                    Button showEntrySource = IconTheme.JabRefIcons.SOURCE.asButton();
+                    showEntrySource.setTooltip(new Tooltip(Localization.lang("Show %0 source", databaseContext.getMode().getFormattedName())));
+                    showEntrySource.setOnMouseClicked(event -> {
+                        this.entryEditor = createEntryEditor();


Why do you need a full entry editor? I think, you can just show the source without full editing capabilities.

JamseBonde007 · 2024-09-29T15:21:08Z

Thank you for your feedback! I've addressed the issues you pointed out and made the necessary changes. Please take a look and let me know if everything looks good now.

koppor · 2024-09-29T15:40:41Z

src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java

+        StringWriter writer = new StringWriter();
+        BibWriter bibWriter = new BibWriter(writer, OS.NEWLINE);
+        FieldWriter fieldWriter = FieldWriter.buildIgnoreHashes(this.preferences.getFieldPreferences());
+        new BibEntryWriter(fieldWriter, new BibEntryTypesManager()).write(entry, bibWriter, type);


Can you try to get the BibEntryTypesManager be passed? It should be available "somewhere" in the call hierarchy. org.jabref.gui.collab.entrychange.PreviewWithSourceTab also has it.

Please add an @implNote that this code is similar to PreviewWithSourceTab#getSourceString.

@Siedlerchr I think it is OK to have code duplication here. Alternatively, we could introduce org.jabref.logic.exporter.BibEntryPrinter having a single method getAsString.

Duplicated code is imho okay, just the EntryTypesManaager as constructor parameter

koppor · 2024-09-29T15:43:26Z

src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java

+        DialogPane dialogPane = new DialogPane();
+        dialogPane.setPrefSize(800, 400);
+        dialogPane.setContent(scrollPane);
+        String title = Localization.lang("%0 source", "Show BibTeX");


"Show BibTeX" is not localized.

Suggested change

String title = Localization.lang("%0 source", "Show BibTeX");

String title = Localization.lang("Show BibTeX source");

koppor · 2024-09-29T15:44:12Z

src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java

@@ -254,6 +271,14 @@ private void styleFetchedListView(CheckListView<CitationRelationItem> listView)
                        vContainer.getChildren().addLast(openWeb);
                    }

+                    Button showEntrySource = IconTheme.JabRefIcons.SOURCE.asButton();
+                    showEntrySource.setTooltip(new Tooltip(Localization.lang("%0 source", databaseContext.getMode().getFormattedName())));


I am not sure if it is really BibLaTeX in this case - the citatoin fetcher always uses the bibtex dialect? Can you check? If unsure, just always use "BibTeX".

koppor · 2024-09-29T15:44:28Z

src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java

@@ -74,6 +87,9 @@ public class CitationRelationsTab extends EntryEditorTab {
    private final BibEntryRelationsRepository bibEntryRelationsRepository;
    private final CitationsRelationsTabViewModel citationsRelationsTabViewModel;
    private final DuplicateCheck duplicateCheck;
+    private EntryEditor entryEditor;
+
+    private StateManager stateManager;


Not used. Remove.

koppor · 2024-09-29T15:45:46Z

src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java

@@ -74,6 +87,9 @@ public class CitationRelationsTab extends EntryEditorTab {
    private final BibEntryRelationsRepository bibEntryRelationsRepository;
    private final CitationsRelationsTabViewModel citationsRelationsTabViewModel;
    private final DuplicateCheck duplicateCheck;
+    private EntryEditor entryEditor;


Not used. Please remove.

It would be really nice if you scrolled through your source code in IntelliJ. That variable should be in light gray meaning: not used. 😅

The thing is that we as reviewers could overlook things. Moreover, if we comment on these kind things, there is no time to check the harder questions... 😇

…07/jabref into feat-for-issue-11826

JamseBonde007 · 2024-09-29T16:48:23Z

Thank you for the feedback and the tips! I've addressed the comments and reviewed the code in IntelliJ. Please take a look at the updates and let me know if everything looks good.

Siedlerchr · 2024-09-29T17:56:02Z

src/main/java/org/jabref/gui/entryeditor/citationrelationtab/CitationRelationsTab.java

        this.taskExecutor = taskExecutor;
        setText(Localization.lang("Citation relations"));
        setTooltip(new Tooltip(Localization.lang("Show articles related by citation")));

-        this.duplicateCheck = new DuplicateCheck(new BibEntryTypesManager());
+        this.entryTypesManager = new BibEntryTypesManager();


You can pass the instance from the caller (see EntryEditor.java)

Originally in "this.duplicateCheck = new DuplicateCheck(new BibEntryTypesManager());" a new instance is created, if I take over the instance from EntryEditor should I leave the original solution for "DuplicateCheck" or move the instance from EntryEditor as well ?

Use the instance from EntryEditor as well. Mabe, this will also fix some bugs...

JamseBonde007 · 2024-09-29T21:59:59Z

Fixed

Add a "view as BibTeX" option before importing an entry from the cita…

a9b5423

…tion relation tab JabRef#11826

koppor requested changes Sep 29, 2024

View reviewed changes

Martin Mochnacký and others added 2 commits September 29, 2024 17:16

Correction of created errors JabRef#11826

bfe9dc9

Merge branch 'main' into feat-for-issue-11826

5f2a0cb

koppor requested changes Sep 29, 2024

View reviewed changes

Martin Mochnacký added 2 commits September 29, 2024 18:44

Fix problems JabRef#11826

e45203d

Merge branch 'feat-for-issue-11826' of https://github.com/JamseBonde0…

4dbfe07

…07/jabref into feat-for-issue-11826

Siedlerchr reviewed Sep 29, 2024

View reviewed changes

Fix: Issuance of an instance JabRef#11826

cbf35b6

Merge branch 'main' into feat-for-issue-11826

ef99fab

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a "view as BibTeX" option before importing an entry from the cita… #11847

Add a "view as BibTeX" option before importing an entry from the cita… #11847

JamseBonde007 commented Sep 29, 2024 •

edited by subhramit

Loading

koppor Sep 29, 2024

koppor Sep 29, 2024

koppor Sep 29, 2024

JamseBonde007 Sep 29, 2024

koppor Sep 29, 2024

koppor Sep 29, 2024

koppor Sep 29, 2024

koppor Sep 29, 2024

koppor Sep 29, 2024

koppor Sep 29, 2024

JamseBonde007 commented Sep 29, 2024

koppor Sep 29, 2024

koppor Sep 29, 2024

Siedlerchr Sep 29, 2024

koppor Sep 29, 2024

koppor Sep 29, 2024

koppor Sep 29, 2024

koppor Sep 29, 2024

JamseBonde007 commented Sep 29, 2024

Siedlerchr Sep 29, 2024

JamseBonde007 Sep 29, 2024 •

edited

Loading

koppor Sep 29, 2024

JamseBonde007 commented Sep 29, 2024

		if (codeArea == null) {
		dialogService.showWarningDialogAndWait(Localization.lang("BibTeX source", databaseContext.getMode().getFormattedName()), Localization.lang("Could not load %0 source", databaseContext.getMode().getFormattedName()));

	String title = Localization.lang("%0 source", "Show BibTeX");
	String title = Localization.lang("Show BibTeX source");

Add a "view as BibTeX" option before importing an entry from the cita… #11847

Are you sure you want to change the base?

Add a "view as BibTeX" option before importing an entry from the cita… #11847

Conversation

JamseBonde007 commented Sep 29, 2024 • edited by subhramit Loading

Mandatory checks

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

Choose a reason for hiding this comment

JamseBonde007 commented Sep 29, 2024

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

JamseBonde007 commented Sep 29, 2024

Choose a reason for hiding this comment

JamseBonde007 Sep 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JamseBonde007 commented Sep 29, 2024

JamseBonde007 commented Sep 29, 2024 •

edited by subhramit

Loading

JamseBonde007 Sep 29, 2024 •

edited

Loading