Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New fetcher #1929

Merged
merged 20 commits into from
Sep 13, 2016
Merged

New fetcher #1929

merged 20 commits into from
Sep 13, 2016

Conversation

tobiasdiez
Copy link
Member

@tobiasdiez tobiasdiez commented Sep 7, 2016

A few new fetcher are introduced:

  • ADS search based
  • MathSciNet
  • Zentralblatt

Also a bit of code was refactored (mainly to facilitate the development of fetcher).

  • Change in CHANGELOG.md described
  • Tests created for changes
  • Screenshots added (for bigger UI changes)
  • Manually tested changed features in running JabRef
  • Check documentation status (Issue created for outdated help page at help.jabref.org?)

@tobiasdiez tobiasdiez changed the title [WIP] New fetcher New fetcher Sep 7, 2016
@tobiasdiez tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Sep 7, 2016
@@ -12,6 +12,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `#
## [Unreleased]

### Changed
- Added fetcher for MathSciNet, zbMATH and Astrophysics Data System
Copy link
Member

Choose a reason for hiding this comment

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

Is it possible to add links to these databases?

@koppor
Copy link
Member

koppor commented Sep 12, 2016

Is it intentional that 4fc8551 and 443f50f went into this commit? Seems to be unrelated.

4fc8551 und 443f50f (besides the minor comments) both LGTM.


import net.sf.jabref.model.entry.BibEntry;

public interface Parser {
Copy link
Member

Choose a reason for hiding this comment

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

Could you add a JavaDoc comment here?

new FieldFormatterCleanup(FieldName.ABSTRACT, new RemoveBracesFormatter()).cleanup(entry);
new FieldFormatterCleanup(FieldName.TITLE, new RemoveBracesFormatter()).cleanup(entry);
new FieldFormatterCleanup(FieldName.AUTHOR, new NormalizeNamesFormatter()).cleanup(entry);
new FieldFormatterCleanup("adsnote", new ClearFormatter()).cleanup(entry);
Copy link
Member

Choose a reason for hiding this comment

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

Could you provide example content in the comment and reason why it is not included? Personally, I keep as much information in the bib files as the styles ignore them nevertheless.

@koppor koppor mentioned this pull request Sep 12, 2016
6 tasks
@tobiasdiez
Copy link
Member Author

Thanks @koppor for reviewing. 4fc8551 is needed because it refactors the BibtexParser to confirm to the new Parser interface. But 443f50f is not really needed (I tried to implement the doPostCleanup method via CleanupWorker but it turned out to be to messy and unneeded).

Will incorporate your feedback.


private static String API_URL = "http://adsabs.harvard.edu/cgi-bin/nph-basic_connect";
private static String API_QUERY_URL = "http://adsabs.harvard.edu/cgi-bin/nph-basic_connect";
Copy link
Member

Choose a reason for hiding this comment

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

make both constants final?

@koppor
Copy link
Member

koppor commented Sep 12, 2016

Went through everything. Besides #1929 everything else are minor comments.

@tobiasdiez tobiasdiez added this to the v3.7 milestone Sep 13, 2016
@tobiasdiez tobiasdiez merged commit 3bd3f6c into JabRef:master Sep 13, 2016
@tobiasdiez tobiasdiez deleted the newFetcher branch September 13, 2016 17:42
zesaro pushed a commit to zesaro/jabref that referenced this pull request Nov 22, 2016
* Refactor search based fetcher based on website and parser

* Refactor BibtexParser

* Refactor cleanup preferences

* Add search-based fetcher for the Astrophysics Data System

* Introduce EntryBasedFetcher

* Add ADS as EntyBasedFetcher

* Add comment about new ADS API

* Add MathSciNet entry based fetcher

* Add MathSciNet search fetcher

* Add zbMath fetcher

* Add Changelog

* Remove header

* Remove more headers

* Include feedback

* Rename GVKParser

* Remove big

* Remove unused imports

* Fix failing GVK tests

* Fix failing tests due to no subscription
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fetcher 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.

3 participants