Skip to content

Group names can now be rendered using LaTeXs commands#1684

Merged
oscargus merged 2 commits into
JabRef:masterfrom
oscargus:propergroupnames
Aug 8, 2016
Merged

Group names can now be rendered using LaTeXs commands#1684
oscargus merged 2 commits into
JabRef:masterfrom
oscargus:propergroupnames

Conversation

@oscargus

@oscargus oscargus commented Aug 5, 2016

Copy link
Copy Markdown
Contributor

Inspired by #1681

  • Change in CHANGELOG.md described
  • Manually tested changed features in running JabRef

@oscargus oscargus added [outdated] type: enhancement component: ui status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers component: groups labels Aug 5, 2016
@tobiasdiez

tobiasdiez commented Aug 5, 2016

Copy link
Copy Markdown
Member

Mhh, I think this is the wrong place to add this functionality. If I name my group \relax (or \beta) then I probably want to keep this name.
But I think it is a good idea to add the Latex-to-Unicode-formatter to determine the name when groups are automatically created.

@oscargus

oscargus commented Aug 5, 2016 via email

Copy link
Copy Markdown
Contributor Author

@stefan-kolb

Copy link
Copy Markdown
Member

Do we really need to allow latex code for group names? Why is normal text not enough? I'd rather not have this option and keep it simple.

@oscargus

oscargus commented Aug 7, 2016

Copy link
Copy Markdown
Contributor Author

See #1681. Names containing accents is the main reason. Already now, the names can contain LaTeX code (except that the reading is flawed, again see #1681). It's just that they are properly rendered now. I do not really see the problem.

@tobiasdiez

tobiasdiez commented Aug 8, 2016

Copy link
Copy Markdown
Member

For me the problem is that the Latex -> Unicode converter is not good enough to handle arbitrary text and I think we may run into problems where names of groups are not properly rendered (i.e. "A or B" in the form "A\B", or similar things with backslash). As a user, I kind of expect that JabRef takes what I give it as a name (especially since I can provide unicode strings, I don't see the reason to support latex accents). Moreover, we might run into big performance problems with groups again.

The idea is nonetheless good. Just put the conversion code in the automatic generation of groups (since there the user has no control over the names of the groups).

@tobiasdiez tobiasdiez removed the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Aug 8, 2016
@oscargus

oscargus commented Aug 8, 2016

Copy link
Copy Markdown
Contributor Author

I see your point, although I'm not fully agreeing with it (I'd rather write $\Delta\Sigma$ than look up the Unicode key and writing \ in anything related to LaTeX expecting it to stay a backslash seems questionable... ;-)), but you solution is fair enough and leads to more or less the same end result. Only question is if BibTeX is bothered by Unicode characters in fields that are ignored anyway? (Yes, there are good reasons to use BibTeX for many people.)

Could you have a look at why the backslashes are removed when reading back the group definitions? (Since you have better insight into the group package.)

@oscargus

oscargus commented Aug 8, 2016

Copy link
Copy Markdown
Contributor Author

I changed it to do the conversion upon generation of automatic groups (and correspondingly changed the CHANGELOG entry).

private final JabRefFrame frame;
private final BasePanel panel;

private static final LatexToUnicodeFormatter FORMATTER = new LatexToUnicodeFormatter();

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 make the formatter a normal variable (initialized directly before the for loop below) instead of static final.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I had it like that first. ;-)

@tobiasdiez

Copy link
Copy Markdown
Member

Ok, just make the fomatter a normal variable and then this can be merged. I will have a look at the serialization / parsing problem as soon as I'm back in Germany (i.e. in a month)

@oscargus oscargus added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Aug 8, 2016
@oscargus oscargus merged commit 086aa37 into JabRef:master Aug 8, 2016
@oscargus oscargus deleted the propergroupnames branch August 8, 2016 12:29
Siedlerchr added a commit to Siedlerchr/jabref that referenced this pull request Aug 9, 2016
* master: (22 commits)
  Do not cite entries without a key in OpenOffice/LibreOffice (JabRef#1682) (JabRef#1683)
  Got rid of unused preferences (JabRef#1672)
  Move labelpattern (JabRef#1626)
  Implementation of shared database support (full system). (JabRef#1451)
  Removed bst from architecture tests JabRef#1699
  Update PULL_REQUEST_TEMPLATE.md
  French localization: Menu: Translation of an empty string
  French localization: Jabref_fr: empty strings translated
  Updated string similarity version (JabRef#1698)
  Minify description of release process
  Announce switch from GPL to MIT in CONTRIBUTING.md and README.md
  Some cleanups to initialize empty MetaData at fewer positions (JabRef#1696)
  Automatic group names are converted from LaTeX to Unicode (JabRef#1684)
  Update ISSUE_TEMPLATE.md (JabRef#1686)
  Removed (false) NPE issue reported by FindBugs
  More Swedish translations (JabRef#1691)
  Updated wiremock version (JabRef#1690)
  Some minor code cleanups (JabRef#1689)
  Removed dependencies of Globals.prefs in some tests (JabRef#1688)
  Lookup BibEntry from ISBN and merge information (JabRef#1621)
  ...

# Conflicts:
#	src/main/java/net/sf/jabref/gui/BasePanel.java
#	src/main/java/net/sf/jabref/importer/ImportMenuItem.java
#	src/main/resources/l10n/JabRef_da.properties
#	src/main/resources/l10n/JabRef_de.properties
#	src/main/resources/l10n/JabRef_en.properties
#	src/main/resources/l10n/JabRef_es.properties
#	src/main/resources/l10n/JabRef_fa.properties
#	src/main/resources/l10n/JabRef_fr.properties
#	src/main/resources/l10n/JabRef_in.properties
#	src/main/resources/l10n/JabRef_it.properties
#	src/main/resources/l10n/JabRef_ja.properties
#	src/main/resources/l10n/JabRef_nl.properties
#	src/main/resources/l10n/JabRef_no.properties
#	src/main/resources/l10n/JabRef_pt_BR.properties
#	src/main/resources/l10n/JabRef_ru.properties
#	src/main/resources/l10n/JabRef_sv.properties
#	src/main/resources/l10n/JabRef_tr.properties
#	src/main/resources/l10n/JabRef_vi.properties
#	src/main/resources/l10n/JabRef_zh.properties
ayanai1 pushed a commit to ayanai1/jabref that referenced this pull request Sep 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component: groups component: ui [outdated] type: enhancement 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.

3 participants