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

source-code-edit breaks file if containing {} #4256

Closed
JoKalliauer opened this issue Aug 10, 2018 · 8 comments
Closed

source-code-edit breaks file if containing {} #4256

JoKalliauer opened this issue Aug 10, 2018 · 8 comments
Labels
component: entry-editor [outdated] type: bug Confirmed bugs or reports that are very likely to be bugs

Comments

@JoKalliauer
Copy link
Contributor

JabRef 5.0-dev--snapshot--2018-07-12--master--dd9b22911
Linux 4.15.0-30-generic amd64
Java 1.8.0_181

Steps to reproduce:

  1. Open JabRefBug.bib.txt
@Article{brown2015tools,
  author    = {Brown, Alan and Long, Fei and Nicholls, Robert A. and Toots, Jaan and Emsley, Paul and Murshudov, Garib},
  title     = {{Tools for macromolecular model building and refinement into electron cryo-microscopy reconstructions}},
  journal   = {Acta Crystallographica, Section D: Biological Crystallography},
  year      = {2015},
  volume    = {71},
  number    = {1},
  pages     = {136--153},
  month     = jan,
  doi       = {10.1107/S1399004714021683},
  file      = {:..\\..\\Literature\\DNA\\Weitere\\Brown_Long_Nicholls_Toots_Emsley_Murshudov_2015_ActaCrystallographicaSectionDBiologicalCrystallography.pdf:PDF},
  groups    = {DNA, Electron Density},
  keywords  = {model building, refinement, electron cryo-microscopy reconstructions, LIBG, rank2},
  publisher = {International Union of Crystallography},
}
  1. goto source-code
    screenshot from 2018-08-10 10-39-37
  2. Delete in the title one of the "{{"
    screenshot from 2018-08-10 10-39-54
  3. File ruined JabRefBug3.bib.txt
@Article{brown2015tools,
  author   = {Brown, Alan and Long, Fei and Nicholls, Robert A. and Toots, Jaan and Emsley, Paul and Murshudov, Garib},
  title    = {Tools for macromolecular model building and refinement into electron cryo-microscopy reconstructions},
  keywords = {rank2},
}

screenshot from 2018-08-10 10-52-14

Log:

Opening: /home/jkalliau/Desktop/JabRefBug.bib
Fix SSL exceptions by accepting ALL certificates
@Siedlerchr
Copy link
Member

Siedlerchr commented Aug 10, 2018

I could reproduce it. You are saving kind of invalid bibtex data.
All braces must be balanced.

If you first remove the { in front and then at the end }, the entry is correct.

By removing one { from the title in front, obviously the last double braces are interpreted as end of field title and end of entry and so the parser constructs this entry:

@article{brown2015tools,
  author = {Brown, Alan and Long, Fei and Nicholls, Robert A. and Toots, Jaan and Emsley, Paul and Murshudov, Garib},
  title = {#Tools##for##macromolecular##model##building##and##refinement##into##electron##cryo-microscopy##reconstructions#}
}

@tobiasdiez
Copy link
Member

We should display a warning instead of messing up the bibtex code in the file, when the parser is not able to make sense of the input or there is another issue. I'll thus mark this as a bug.

@tobiasdiez tobiasdiez added [outdated] type: bug Confirmed bugs or reports that are very likely to be bugs component: entry-editor labels Aug 10, 2018
@JoKalliauer
Copy link
Contributor Author

@Siedlerchr : I wanted to change title = {{Some Title}}, to title = {Some Title}, Therfore I first delete the left{ and then the right}, since I read left to right I also edit left to right. But immediately without saving, without moving to another field the bibtex-entry get messed up, before I could even delete the closing bracket.

The only solution is to first remove the closing bracket and then the opening bracket, but that's not how somebody would edit.

@frasca80
Copy link
Contributor

frasca80 commented Jan 9, 2019

Can I work on this issue?
I had a look at the code: a quick solution would be to trigger the BibTex parser on focus out of the text area instead of on value change event as it is now.
In fact, following the steps to reproduce the issue, now the parser is invoked just after the removal of the first bracket '{' preventing the user to finish his/her edit action which would include also the removal of the second closing bracket '}'. This leads the parser to an unwanted truncation of the BibTex source string.

@Siedlerchr
Copy link
Member

@frasca80 Sure, go ahead! Your solution sound good

@tobiasdiez
Copy link
Member

@frasca80 Sure, feel free to work on it. A problem I see with your suggested solution is that leaving the source code editor after removing the initial { still leads to a corrupted entry; this shouldn't be the case. The ParserResult has a hasWarnings method that shows if the parsing was succussful. I would thus suggest to only update the entry with the new value if the parser does not report any warnings (even better: display the warning to the user so that he is aware of the fact that the current code is not valid).

@frasca80
Copy link
Contributor

frasca80 commented Jan 9, 2019

@frasca80 Sure, feel free to work on it. A problem I see with your suggested solution is that leaving the source code editor after removing the initial { still leads to a corrupted entry; this shouldn't be the case. The ParserResult has a hasWarnings method that shows if the parsing was succussful. I would thus suggest to only update the entry with the new value if the parser does not report any warnings (even better: display the warning to the user so that he is aware of the fact that the current code is not valid).

@tobiasdiez you're right but unfortunately if the user removes only the first { the parser does not raise any warning since it stops to what it considers the end of the entry, that is the second encountered } (see title = {Tools for macromolecular model building and refinement into electron cryo-microscopy reconstructions}},). For this reason the parser ignores the remaining part of the source.
This remaining part however is assigned to BibDatabase::epilog, do you think it would be correct, at the end of the parsing, to do some check on the epilog in order to eventually raise a warning? e.g. check if epilog contains a field in the form name={content}

@Siedlerchr
Copy link
Member

Thanks to @frasca80 this issue should now be fixed in the latest master!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: entry-editor [outdated] type: bug Confirmed bugs or reports that are very likely to be bugs
Projects
Archived in project
Development

No branches or pull requests

4 participants