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

Fix for issue 8747: date field change causes an uncaught exception #9314

Closed
wants to merge 9 commits into from

Conversation

huangderek02
Copy link

@huangderek02 huangderek02 commented Oct 27, 2022

Fixes #8747

Short Description

In library mode "biblatex", when user edits the entry's "date" field with an invalid input and clicks somewhere, an uncaught exception of null pointer occurs.

Checklist

  • 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 developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • 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.

UI changes (takes time to load gif images)

  • Before

  • Type invalid inputs: nothing happen

  • Click somewhere: an exception occured;

  • Press Enter: input disappears
    312775146_691460315939346_8658484514782598519_n

  • After

  • Type invalid inputs: show warning icon

  • Click somewhere: nothing happens

  • Press Enter: input disappears
    313129752_1170515957176323_7279031655860662192_n

312829199_3262902150588889_342121938428642385_n

@Siedlerchr
Copy link
Member

Siedlerchr commented Oct 27, 2022

That is a nice improvement. Regarding invalid inputs, as biblatex supports a couple of more dates, see also #9310 ) can you try to integrate this with JabRef's date time parsing class?

And please take a look at the reviewdog output (Files changed tab)

@Siedlerchr Siedlerchr added the status: changes required Pull requests that are not yet complete label Oct 28, 2022
@xianghao-wang
Copy link
Contributor

That is a nice improvement. Regarding invalid inputs, as biblatex supports a couple of more dates, see also #9310 ) can you try to integrate this with JabRef's date time parsing class?

And please take a look at the reviewdog output (Files changed tab)

Hello, could you help have a look at the new change by adding Date integration to the default string converter? I think the string converter provided by DateEditorViewModel already have worked with Date class code.

@@ -36,7 +36,7 @@ public TemporalAccessor fromString(String string) {
if (StringUtil.isNotBlank(string)) {
try {
return dateFormatter.parse(string);
} catch (DateTimeParseException exception) {
} catch (Exception exception) {
Copy link
Member

Choose a reason for hiding this comment

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

Why did you change this to a generic exception? Exception should be always the most specfic one

@Siedlerchr
Copy link
Member

I tested your PR and it looks good so far. Please have a look at the failing checkstyle tests and also the exception handling.
I cannot push to your repo so I can't do this myself

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
entry-editor status: changes required Pull requests that are not yet complete
Projects
None yet
Development

Successfully merging this pull request may close these issues.

date field with non date data generates error
4 participants