-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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 #3805: dash correctly terminates commands in layout files #4137
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides micro comments: LGTM
CHANGELOG.md
Outdated
@@ -31,6 +31,7 @@ We refer to [GitHub issues](https://github.com/JabRef/jabref/issues) by using `# | |||
- We fixed an issue where in rare cases entries where overlayed in the main table. https://github.com/JabRef/jabref/issues/3281 | |||
- We fixed an issue where selecting a group messed up the focus of the main table / entry editor. https://github.com/JabRef/jabref/issues/3367 | |||
- We fixed an issue where composite author names were sorted incorrectly. https://github.com/JabRef/jabref/issues/2828 | |||
- We fixed an issue where commands followed by `-` didn't worked. [#3805](https://github.com/JabRef/jabref/issues/3805) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please change "worked" into "work".
@@ -33,7 +33,7 @@ public Layout(List<StringInt> parsedEntries, LayoutFormatterPreferences prefs) { | |||
for (StringInt parsedEntry : parsedEntries) { | |||
switch (parsedEntry.i) { | |||
case LayoutHelper.IS_LAYOUT_TEXT: | |||
case LayoutHelper.IS_SIMPLE_FIELD: | |||
case LayoutHelper.IS_SIMPLE_COMMAND: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please double check tabs vs. spaces there and the line before/after
@@ -109,7 +109,7 @@ public LayoutEntry(StringInt si, LayoutFormatterPreferences prefs) { | |||
case LayoutHelper.IS_LAYOUT_TEXT: | |||
text = si.s; | |||
break; | |||
case LayoutHelper.IS_SIMPLE_FIELD: | |||
case LayoutHelper.IS_SIMPLE_COMMAND: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the extra indent? (applies for all case statements in this file)
Fixes #3805. Now commands terminated by a hyphen (like
\edition-
) work correctly.Way back in 2011, Morten added the extra check for hyphens (commit 392bc7a). However, I'm not aware of any field that has a dash in it, so I'm not sure what was the reason for the change (I couldn't find an old bug report).
Also refactored a bit the layout tests.