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

Use regexp to remove non-ASCII characters from DOI and inform user when data for valid DOI does not exist #8127 #8228

Merged
merged 9 commits into from
Nov 13, 2021

Conversation

mrcstan
Copy link
Contributor

@mrcstan mrcstan commented Nov 8, 2021

Use single regexp to remove white-space and non-ASCII characters in DOI to fixes #8127. Further inform the user that "No DOI data exists" if DOI is valid and unable to find the article. Works for cases where DOI.findInText does not #8227. For example, the DOI "https : / / doi.org / 10 / gf4gqc" with the quote removed will result in an error not capture by Jabref itself. The reason is that the pattern used by DOI.findInText to extract DOI is not comprehensive. DOI.findInText is only tested in DOITest but not used elsewhere in the source code. However, DOI.parse is used in both DoiFetcher and CompositeIdFetcher, so it is probably better to improve DOI.parse.

With DOI.findInText:
image

With this PR:
image

With this PR, user is informed that no data exists if the DOI is valid but no data is found online
image

  • 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 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.

…from DOI

Use regexp to remove non-ASCII, control and non-printable characters from DOI

Added language keys to JabRef_en.properties and updated CHANGELOG.md

 Fixed code style issue

Added change to CHANGELOG.md

Before merging with origin

Use regexp to remove non-ASCII, control and non-printable characters from DOI

Squash commits before pulling from origin

Use regexp to remove non-ASCII, control and non-printable characters from DOI

Squash commits before pulling from origin

Added language keys to JabRef_en.properties and updated CHANGELOG.md

Use regexp to remove non-ASCII, control and non-printable characters from DOI
…from DOI

Squash commits before pulling from origin

Squash all commits for this branch
@mrcstan mrcstan force-pushed the fix-for-issue-#8127 branch from 893ceb5 to 76ecf91 Compare November 9, 2021 07:42
@mrcstan mrcstan marked this pull request as ready for review November 9, 2021 07:52
@mrcstan mrcstan changed the title Use regexp to remove non-ASCII, control and non-printable characters from DOI #8127 Use regexp to remove non-ASCII characters from DOI and inform user when data for valid DOI does not exist #8127 Nov 10, 2021
Copy link
Member

@koppor koppor left a comment

Choose a reason for hiding this comment

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

Small comment - other than that: LGTM!

src/main/java/org/jabref/model/entry/identifier/DOI.java Outdated Show resolved Hide resolved
@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Nov 12, 2021
Copy link
Member

@Siedlerchr Siedlerchr left a comment

Choose a reason for hiding this comment

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

Thanks, lgtm now!

@koppor koppor merged commit 2929900 into JabRef:main Nov 13, 2021
@koppor koppor removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Nov 13, 2021
Siedlerchr added a commit that referenced this pull request Nov 19, 2021
* upstream/main: (181 commits)
  Add of ADRs 22 and 23 (#8256)
  [Bot] Update CSL styles (#8245)
  Replace styfle/cancel-workflow-action@0.9.1 by GitHub's "concurrency" feature (#8243)
  Bump gittools/actions from 0.9.10 to 0.9.11 (#8248)
  Bump commons-cli from 1.4 to 1.5.0 (#8250)
  Bump byte-buddy-parent from 1.12.0 to 1.12.1 (#8249)
  Bump antlr4 from 4.9.2 to 4.9.3 (#8251)
  Bump archunit-junit5-api from 0.21.0 to 0.22.0 (#8252)
  Fix search: NOT binds more than AND (#8241)
  New Crowdin updates (#8240)
  Make PdfGrobiImporterTest as FetcherTest
  Oobranch g : add actions (#7792)
  Fix mixed CRLF / CR (#8238)
  Fix "Library has changed externally" with CRLF markers (#8239)
  Fix for issue 8198, 8133 (#8229)
  Added more unit tests in AuthorTest (#8214)
  Add confirmation dialog for empty entries in JabRef (#8218)
  Fix entry editor column visibility (#8232)
  Use regexp to remove non-ASCII characters from DOI and inform user when data for valid DOI does not exist #8127 (#8228)
  Fix exception for search flags (#8237)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid DOI should not show "Connection error"
3 participants