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

Write groups under tag group instead of groupstree to enhance compatibility with previous versions #2704

Merged
merged 2 commits into from
Apr 6, 2017

Conversation

tobiasdiez
Copy link
Member

Right now, files written with 4.0 cannot be opened in older versions. The new format of the groups leads to a NPE.
As a way out, I changed the tag for the groups tree from groupstree to group (groups was already used by previous version way back). Thus older versions completely ignore the group tree and no longer try to parse it, so no NPE is thrown. On the downside, however, the groups tree is deleted upon save in the older version. Still a bit better than a crashing Jabref in my opinion.

  • 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 Apr 2, 2017
@Siedlerchr
Copy link
Member

On the downside, however, the groups tree is deleted upon save in the older version. Still a bit better than a crashing Jabref in my opinion.

I wonder why JabRef does not simply ignore the groups Tree then and deletes it on save.

@tobiasdiez tobiasdiez changed the title Write groups under tag group instead of groupstree to hence compatibility with previous versions Write groups under tag group instead of groupstree to enhence compatibility with previous versions Apr 3, 2017
@lenhard
Copy link
Member

lenhard commented Apr 5, 2017

@tobiasdiez Even the old versions from 3.1 or 3.2 onward (not sure which) should not delete the new groups tree, since at some point we implemented the feature that JabRef does no longer delete random text from the bib file but keeps it. Anything prior to v3.0 will certainly delete it.

Anyway, I think this is a good compromise. Then people with older versions can still open the file and when they wonder that the groups tree is not correct (or gone), we can tell them to upgrade.

I just think you should use groups instead of group as the key in the meta data, since this is normally not about a tree consisting of a single group, but of multiple groups.

@lenhard lenhard changed the title Write groups under tag group instead of groupstree to enhence compatibility with previous versions Write groups under tag group instead of groupstree to enhance compatibility with previous versions Apr 5, 2017
@tobiasdiez
Copy link
Member Author

tobiasdiez commented Apr 5, 2017

Well, even 3.8.2 deletes the group tag, probably since it is an unknown metadata item.

I could change group to groups but it was already used in a very (?) old JabRef version...not sure which.

@lenhard
Copy link
Member

lenhard commented Apr 5, 2017

I see, with metadata the serialization might be different.

Hm, if it has been used before, we probably should not use groups. Since this name goes into the bib files of many people and will annoy us if we change it later, let us discuss a good version in the next devcall.

@koppor
Copy link
Member

koppor commented Apr 6, 2017

Decision: grouping

Alternatives: grouping, groupies, group, groups

@lenhard lenhard mentioned this pull request Apr 6, 2017
3 tasks
@tobiasdiez tobiasdiez merged commit 27d3aea into master Apr 6, 2017
@tobiasdiez tobiasdiez deleted the groupWriteDifferent branch April 6, 2017 13:50
Siedlerchr added a commit that referenced this pull request Apr 6, 2017
* upstream/master:
  Implement #1359: collect telemetry (#2283)
  Write groups under tag `group` instead of `groupstree` to enhance compatibility with previous versions (#2704)
  Avoid conversion of single underscores (#2711)
  Fix #628: implement hierarchical keywords (#2703)
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.

4 participants