Skip to content

Fix #2427: Arxiv fetcher works with prefix#2428

Merged
koppor merged 1 commit into
masterfrom
arxivPrefix
Dec 24, 2016
Merged

Fix #2427: Arxiv fetcher works with prefix#2428
koppor merged 1 commit into
masterfrom
arxivPrefix

Conversation

@tobiasdiez

Copy link
Copy Markdown
Member
  • 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?)
  • If you changed the localization: Did you run gradle localizationUpdate?

@tobiasdiez tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Dec 22, 2016

private Optional<ArXivEntry> searchForEntryById(String identifier) throws FetcherException {
identifier = identifier.replace("arxiv:", "");
identifier = identifier.replace("arXiv:", "");

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.

Can you just replace this via a regex? /arxiv:?/i

@simonharrer simonharrer Dec 22, 2016

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I would vote for an ArXivID class, similar to the DOI one which can handle such a case.

assertEquals(Optional.of(new URL("http://arxiv.org/pdf/1603.06570v1")), finder.findFullText(entry));
}

@Test

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.

I would omit the comment here. It is a test for expected behavior. Doesnt matter which bug/problem it fixed.

assertEquals(Optional.of(sliceTheoremPaper), finder.performSearchById("1405.2249"));
}

@Test

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.

Same as above

@Test
// Test for https://github.com/JabRef/jabref/issues/2427
public void findByEprintWithPrefix() throws IOException {
entry.setField("eprint", "arXiv:1603.06570");

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.

Remove empty line

assertEquals(Optional.of(sliceTheoremPaper), finder.performSearchById("1405.2249"));
}

@Test

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.

Remove url

@koppor koppor merged commit 8be62e0 into master Dec 24, 2016
@koppor

koppor commented Dec 24, 2016

Copy link
Copy Markdown
Member

I merged it and will do the requested changes now to get this into 3.8.2.

@koppor koppor deleted the arxivPrefix branch December 24, 2016 13:31
koppor added a commit that referenced this pull request Dec 24, 2016
@tobiasdiez tobiasdiez mentioned this pull request Dec 28, 2016
6 tasks
tobiasdiez added a commit that referenced this pull request Jan 15, 2017
* Create ArXivIdentifier class

* Add arXiv ID-based fetcher (and tests)

* Fix style

* Add end of file line breaks
Siedlerchr added a commit that referenced this pull request Jan 18, 2017
* upstream/master:
  Save deletion of current searchquery (#2469)
  Update dev dependencies (mockito, wiremock, assertj)
  Update BouncyCastle (1.55->1.56), ANTRL4 (4.5.3->4.6), jsoup (1.10.1->1.10.2)
  Group all checker which only check the value of one field (#2437)
  Follow up #2428 (#2438)
  Fix conversion of "'n" and "\'{n}" from LaTeX to Unicode (#2464)
  Fix failing tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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.

5 participants