Skip to content

Lookup BibEntry from ISBN and merge information#1621

Merged
oscargus merged 15 commits into
JabRef:masterfrom
oscargus:isbnlookup
Aug 7, 2016
Merged

Lookup BibEntry from ISBN and merge information#1621
oscargus merged 15 commits into
JabRef:masterfrom
oscargus:isbnlookup

Conversation

@oscargus

Copy link
Copy Markdown
Contributor

Should fix the translations and just add ISBN where applicable.

  • Change in CHANGELOG.md described

@@ -50,46 +51,75 @@ public class MergeEntryDOIDialog extends JDialog {
private final BasePanel panel;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename class to MergeEntryDialog?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is already one with that name. :-)

@oscargus

Copy link
Copy Markdown
Contributor Author

I pass on rewriting to the new interface at the moment, but there is a rewrite of the code to make it very simple to add merging based on other fields.

I'll make one small fix (add ISBN as a FieldExtra) and then it is ready for review.

@oscargus oscargus changed the title [WIP] Lookup BibEntry from ISBN and merge information Lookup BibEntry from ISBN and merge information Jul 25, 2016
@oscargus oscargus added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jul 25, 2016
@oscargus oscargus force-pushed the isbnlookup branch 2 times, most recently from 8992bc5 to 11eb2c7 Compare July 26, 2016 11:24
@oscargus

Copy link
Copy Markdown
Contributor Author

With the current code structure it would be simple to allow marking of several entries and loop through them. Main issue I would guess is that there is no way to effectively canceling this operation at the moment, one will just have to go through everything and it is not straightforward to solve that part.

Should I still implement it?

@simonharrer

Copy link
Copy Markdown
Contributor

I think the looping through multiple entries should be done in another PR. And we have to think about how to handle this in general. The user should be able to abort such actions if they require user interaction per entry.

@oscargus

oscargus commented Aug 2, 2016

Copy link
Copy Markdown
Contributor Author

I agree that there should be some better mechanism for canceling multiple entries.

The conflicts are all for translations. I wait with rebasing until this gets an OK to be merged.

Btw, I added merging from ArXiv as well to this PR.

@simonharrer

Copy link
Copy Markdown
Contributor

To me, it looks good. But I think the IdBasedFetcher would be really helpful to implement here. However, this can be done in another PR.

} else {
JOptionPane.showMessageDialog(frame(),
Localization.lang("This operation requires exactly one item to be selected."),
Localization.lang("Merge entry with %0 information", "DOI/ISBN"),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"with fetched information" ? since DOI, ISBN and now eprint is supported.

@tobiasdiez

Copy link
Copy Markdown
Member

In general, this PR looks good to me 👍 Just a few minor remarks.

By the way, I still don't get why there are two different dialogs for comparing/merging entries (MergeEntryDialog vs the new MergeFetchedEntryDialog)

@oscargus

oscargus commented Aug 2, 2016 via email

Copy link
Copy Markdown
Contributor Author

private boolean isAnyFieldSetForSelectedEntry(List<String> fieldnames) {
if (panel.getMainTable().getSelectedRowCount() == 1) {
BibEntry entry = panel.getMainTable().getSelected().get(0);
for (String fieldname : fieldnames) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simple one liner: ! Collections.disjoint(list1, list2);
From the apidoc: disjoint (a, b) Returns true if the two specified collections have no elements in common.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! There should have been an easier way...

@oscargus oscargus merged commit d5c1fd9 into JabRef:master Aug 7, 2016
@oscargus oscargus deleted the isbnlookup branch August 7, 2016 18:47
Siedlerchr added a commit to Siedlerchr/jabref that referenced this pull request Aug 9, 2016
* master: (22 commits)
  Do not cite entries without a key in OpenOffice/LibreOffice (JabRef#1682) (JabRef#1683)
  Got rid of unused preferences (JabRef#1672)
  Move labelpattern (JabRef#1626)
  Implementation of shared database support (full system). (JabRef#1451)
  Removed bst from architecture tests JabRef#1699
  Update PULL_REQUEST_TEMPLATE.md
  French localization: Menu: Translation of an empty string
  French localization: Jabref_fr: empty strings translated
  Updated string similarity version (JabRef#1698)
  Minify description of release process
  Announce switch from GPL to MIT in CONTRIBUTING.md and README.md
  Some cleanups to initialize empty MetaData at fewer positions (JabRef#1696)
  Automatic group names are converted from LaTeX to Unicode (JabRef#1684)
  Update ISSUE_TEMPLATE.md (JabRef#1686)
  Removed (false) NPE issue reported by FindBugs
  More Swedish translations (JabRef#1691)
  Updated wiremock version (JabRef#1690)
  Some minor code cleanups (JabRef#1689)
  Removed dependencies of Globals.prefs in some tests (JabRef#1688)
  Lookup BibEntry from ISBN and merge information (JabRef#1621)
  ...

# Conflicts:
#	src/main/java/net/sf/jabref/gui/BasePanel.java
#	src/main/java/net/sf/jabref/importer/ImportMenuItem.java
#	src/main/resources/l10n/JabRef_da.properties
#	src/main/resources/l10n/JabRef_de.properties
#	src/main/resources/l10n/JabRef_en.properties
#	src/main/resources/l10n/JabRef_es.properties
#	src/main/resources/l10n/JabRef_fa.properties
#	src/main/resources/l10n/JabRef_fr.properties
#	src/main/resources/l10n/JabRef_in.properties
#	src/main/resources/l10n/JabRef_it.properties
#	src/main/resources/l10n/JabRef_ja.properties
#	src/main/resources/l10n/JabRef_nl.properties
#	src/main/resources/l10n/JabRef_no.properties
#	src/main/resources/l10n/JabRef_pt_BR.properties
#	src/main/resources/l10n/JabRef_ru.properties
#	src/main/resources/l10n/JabRef_sv.properties
#	src/main/resources/l10n/JabRef_tr.properties
#	src/main/resources/l10n/JabRef_vi.properties
#	src/main/resources/l10n/JabRef_zh.properties
ayanai1 pushed a commit to ayanai1/jabref that referenced this pull request Sep 5, 2016
* Lookup BibEntry from ISBN and merge information

* Added support for ArXiv eprint merging
Sign up for free to join this conversation on GitHub. Already have an account? Sign in 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.

4 participants