Allow addition of an entry by its Id (e.g., DOI, ISBN) - fixes #4183 #550 #5755

CarodePourtales · 2019-12-16T23:17:14Z

Creation of a button which opens a dialog.
Only one textfield for an id
Regex on the id to know the fetcher
3 buttons : cancel, look the source and generate
#4183 #550
-->

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 bigger UI changes)
Checked documentation: Is the information available and up to date? If not: Issue created at https://github.com/JabRef/user-documentation/issues.

…e un dialog avec un textfield pour un id dont le Fetcher est trouvé par Regex.

Travail sur l'issue #4183 et #550 pour implémenter un bouton qui ouvr…

avancée issue 4183 : reste travail sur pattern, beauté affichage

koppor

Some first comments.

Could you please provide screenshots so that there is no need to run JabRef locally to provide a timely feedback?

koppor · 2019-12-18T05:18:06Z

src/main/java/org/jabref/gui/EntryType.fxml

@@ -12,35 +12,43 @@
 <?import javafx.scene.layout.GridPane?>
 <?import javafx.scene.layout.RowConstraints?>
 <?import javafx.scene.layout.VBox?>
+


Why this reformatting in the file? Can't the file be kept as is?

koppor · 2019-12-18T05:18:50Z

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

+
+ final BibEntry entry = result.get();
+
+ String entryString = "Found with : \n" + fetcherName + "\n\n with ID : \n" + entry.getId() + "\n\n Title : \n" + entry.getTitle() + "\n\n Publication date : \n" + entry.getPublicationDate();


Please localize that. I think, wrapping in Localizaion.lang is the thing you are searching for.

Please no spaces before the :.

koppor · 2019-12-18T05:19:46Z

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

+import org.jabref.logic.importer.IdBasedFetcher;
+import org.jabref.logic.importer.NoFetcherFoundException;
+import org.jabref.logic.importer.WebFetchers;
+import org.jabref.logic.importer.fetcher.*;


Your workspace doesn't seem to be fully setup according to https://github.com/JabRef/jabref/wiki/Guidelines-for-setting-up-a-local-workspace

We don't have imports with *. Please ensure that you followed all steps given there.

koppor · 2019-12-18T05:29:07Z

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

+ }
+ } else {
+ // Regenerate CiteKey of imported BibEntry
+ new BibtexKeyGenerator(basePanel.getBibDatabaseContext(), prefs.getBibtexKeyPatternPreferences()).generateAndSetKey(entry);


The cleanup for the bibtex key should be used here - don't we have such a cleanupper? (Reason: consistency with other JAbRef Code).

koppor · 2019-12-18T05:32:18Z

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

+ }
+ }
+
+ private class FetcherWorker extends Task<Optional<BibEntry>> {


Doesn't JabRef have an infrastructure for background tasks somehow? Maybe, this is a TODO. The current code here feels a bit quickly hacked. At least, this nested class should be factored out to a separate class.

The BackgroundTask class is perfectly made for such cases.

koppor · 2019-12-18T05:35:09Z

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

- } else {
- EntryTypeView typeChoiceDialog = new EntryTypeView(jabRefFrame.getCurrentBasePanel(), dialogService, preferences);
- EntryType selectedType = typeChoiceDialog.showAndWait().orElse(null);
+ if (fromID){


Boolean parameters should not be used. This smells for a second method (or enums - see http://media.pragprog.com/titles/javacomp/split.pdf)

I think this should actually be a completely new class.

koppor · 2019-12-18T05:36:25Z

src/main/resources/l10n/JabRef_fr.properties

@@ -91,6 +91,8 @@ Automatically\ remove\ exact\ duplicates=Supprimer automatiquement les doublons

 AUX\ file\ import=Importation de fichier AUX

+author= Auteur


Thin, "author" is not used in the code. This string is obsolete.

Please translate full sentences instead of part of sentences.

koppor · 2019-12-18T05:36:36Z

src/main/resources/l10n/JabRef_en.properties

@@ -1258,6 +1269,7 @@ Your\ style\ file\ specifies\ the\ character\ format\ '%0',\ which\ is\ undefine
 Your\ style\ file\ specifies\ the\ paragraph\ format\ '%0',\ which\ is\ undefined\ in\ your\ current\ OpenOffice/LibreOffice\ document.=Your style file specifies the paragraph format '%0', which is undefined in your current OpenOffice/LibreOffice document.

 Searching...=Searching...
+Adding...= Adding ...


Please do not have spaces before/after a translation.

Adding...=Adding...

koppor · 2019-12-18T05:40:14Z

src/main/resources/l10n/JabRef_fr.properties

@@ -394,6 +396,8 @@ Formatter\ name=Nom de formateur

 found\ in\ AUX\ file=trouvées dans le fichier AUX

+FromID= Entrez l'identifiant comme un ISBN, DOI, ou Arxiv dans le champ ci-dessous


The key has to be the full sentence in English.

koppor · 2019-12-18T05:40:43Z

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

@@ -259,6 +260,7 @@ private void setupActions() {

 actions.put(Actions.SAVE_SELECTED_AS_PLAIN, saveAction::saveSelectedAsPlain);

+


Please not two empty lines between statements.

tobiasdiez

Thanks a lot for your contribution. The code looks already pretty good. I've some suggestions concerning the general architecture.

tobiasdiez · 2019-12-31T16:02:38Z

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

+ }
+ }
+
+ private class FetcherWorker extends Task<Optional<BibEntry>> {


The BackgroundTask class is perfectly made for such cases.

tobiasdiez · 2019-12-31T16:05:52Z

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

- } else {
- EntryTypeView typeChoiceDialog = new EntryTypeView(jabRefFrame.getCurrentBasePanel(), dialogService, preferences);
- EntryType selectedType = typeChoiceDialog.showAndWait().orElse(null);
+ if (fromID){


I think this should actually be a completely new class.

tobiasdiez · 2019-12-31T16:07:17Z

src/main/java/org/jabref/logic/importer/NoFetcherFoundException.java

+
+import org.jabref.JabRefException;
+
+public class NoFetcherFoundException extends RuntimeException {


Not sure if this really needs an exception. Normally, you don't want to use an exception to control the flow of the execution and "No fetcher found" doesn't sound like an abnormal situation but more like a special situation in a normal workflow.

tobiasdiez · 2019-12-31T16:09:40Z

src/main/java/org/jabref/logic/importer/WebFetchers.java

 */
+ public static HashMap<Predicate<String>,IdBasedFetcher> getHashMapPredicateIdBasedFetchers(ImportFormatPreferences importFormatPreferences) {


I think it is better if the IdBasedFetcher interface is extended by a method boolean matchIdentifier(string identifier), which then is implemented by each fetcher. In this way, the fetcher itself is responsible to decide which identifier it is able to match.

koppor · 2020-02-28T07:40:41Z

@CarodePourtales May I ask whether you find time to complete this pull request? We would really like to include that feature in JabRef. Is there something that we can do to support you to adress the comments?

tobiasdiez · 2020-04-10T15:35:27Z

@CarodePourtales any update on this? It would be sad if your good work would be lost. Since only a few polishing changes are required, this shouldn't take long. Thanks!

koppor · 2020-08-31T19:10:44Z

On JabCon 2020, we decided to freeze this PR and focus on the other opened PRs. We will come back to this issue as soon as our resources are free for it.

60bf7d5 Add encyclopedia type to wikipedia-templates.csl (#5778) 031afe1 Update and rename dependent/organization-studies.csl to organization-… (#5779) 7ed71e7 Update harvard-newcastle-university.csl (#5765) 46bab91 Update annals-of-oncology.csl (#5760) 6158ae6 Create research-in-plant-disease.csl (#5738) 04422a8 Create chemmedchem.csl (#5753) 7c11521 Create clinical-kidney-journal.csl (#5749) e7ee983 Create kit-karlsruher-institut-fur-technologie-germanistik-ndl-neuere… (#5729) 96a1106 Update article format for STM journal (#5755) a4ca057 Update historia-scribere.csl (#5748) git-subtree-dir: buildres/csl/csl-styles git-subtree-split: 60bf7d5

CarodePourtales and others added 4 commits December 11, 2019 14:11

Travail sur l'issue #4183 et #550 pour implémenter un bouton qui ouvr…

ca4e025

…e un dialog avec un textfield pour un id dont le Fetcher est trouvé par Regex.

Merge pull request #1 from CarodePourtales/issue4183

038911d

Travail sur l'issue #4183 et #550 pour implémenter un bouton qui ouvr…

avancée issue 4183 : reste travail sur pattern, beauté affichage

b38d442

Merge pull request #2 from CarodePourtales/issue+4183

b30c3de

avancée issue 4183 : reste travail sur pattern, beauté affichage

koppor changed the title ~~#4183 #550~~ Allow addition of an entry by its Id (e.g., DOI, ISBN) - fixes #4183 #550 Dec 17, 2019

koppor requested changes Dec 18, 2019

View reviewed changes

koppor mentioned this pull request Dec 18, 2019

Travail sur l'issue #4183 et #550 pour implémenter un bouton qui ouvr… #5733

Closed

4 tasks

koppor assigned CarodePourtales Dec 20, 2019

koppor changed the title ~~Allow addition of an entry by its Id (e.g., DOI, ISBN) - fixes #4183 #550~~ [WIP] Allow addition of an entry by its Id (e.g., DOI, ISBN) - fixes #4183 #550 Dec 20, 2019

tobiasdiez requested changes Dec 31, 2019

View reviewed changes

koppor force-pushed the master branch 5 times, most recently from b8ef7b7 to 21c6e5e Compare March 4, 2020 17:02

koppor assigned Siedlerchr and unassigned CarodePourtales Mar 31, 2020

tobiasdiez marked this pull request as draft April 17, 2020 15:58

koppor changed the title ~~[WIP] Allow addition of an entry by its Id (e.g., DOI, ISBN) - fixes #4183 #550~~ Allow addition of an entry by its Id (e.g., DOI, ISBN) - fixes #4183 #550 Apr 23, 2020

koppor closed this Aug 31, 2020

koppor unassigned Siedlerchr Aug 31, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow addition of an entry by its Id (e.g., DOI, ISBN) - fixes #4183 #550 #5755

Allow addition of an entry by its Id (e.g., DOI, ISBN) - fixes #4183 #550 #5755

CarodePourtales commented Dec 16, 2019 •

edited by calixtus

Loading

koppor left a comment

koppor Dec 18, 2019

koppor Dec 18, 2019

koppor Dec 18, 2019

koppor Dec 18, 2019

koppor Dec 18, 2019

tobiasdiez Dec 31, 2019

koppor Dec 18, 2019

tobiasdiez Dec 31, 2019

koppor Dec 18, 2019

koppor Dec 18, 2019

koppor Dec 18, 2019

koppor Dec 18, 2019

tobiasdiez left a comment

tobiasdiez Dec 31, 2019

tobiasdiez Dec 31, 2019

tobiasdiez Dec 31, 2019

tobiasdiez Dec 31, 2019

koppor commented Feb 28, 2020

tobiasdiez commented Apr 10, 2020

koppor commented Aug 31, 2020


		final BibEntry entry = result.get();

		String entryString = "Found with : \n" + fetcherName + "\n\n with ID : \n" + entry.getId() + "\n\n Title : \n" + entry.getTitle() + "\n\n Publication date : \n" + entry.getPublicationDate();

		@@ -91,6 +91,8 @@ Automatically\ remove\ exact\ duplicates=Supprimer automatiquement les doublons

		AUX\ file\ import=Importation de fichier AUX

		author= Auteur

		@@ -394,6 +396,8 @@ Formatter\ name=Nom de formateur

		found\ in\ AUX\ file=trouvées dans le fichier AUX

		FromID= Entrez l'identifiant comme un ISBN, DOI, ou Arxiv dans le champ ci-dessous

		@@ -259,6 +260,7 @@ private void setupActions() {

		actions.put(Actions.SAVE_SELECTED_AS_PLAIN, saveAction::saveSelectedAsPlain);


		import org.jabref.JabRefException;

		public class NoFetcherFoundException extends RuntimeException {

		*/
		public static HashMap<Predicate<String>,IdBasedFetcher> getHashMapPredicateIdBasedFetchers(ImportFormatPreferences importFormatPreferences) {

Allow addition of an entry by its Id (e.g., DOI, ISBN) - fixes #4183 #550 #5755

Allow addition of an entry by its Id (e.g., DOI, ISBN) - fixes #4183 #550 #5755

Conversation

CarodePourtales commented Dec 16, 2019 • edited by calixtus Loading

koppor 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

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

tobiasdiez 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

koppor commented Feb 28, 2020

tobiasdiez commented Apr 10, 2020

koppor commented Aug 31, 2020

CarodePourtales commented Dec 16, 2019 •

edited by calixtus

Loading