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

Make wrap fields also visible in entry editor #6315

Merged
merged 6 commits into from
May 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,12 @@ Note that this project **does not** adhere to [Semantic Versioning](http://semve
- We fixed the display of icon both in the main table and linked file editor. [#6169](https://github.com/JabRef/jabref/issues/6169)
- We fixed the paste entry command in the menu and toolbar, that did not do anything. [#6293](https://github.com/JabRef/jabref/issues/6293)
- We fixed an issue where the windows installer did not create an entry in the start menu [bug report in the forum](https://discourse.jabref.org/t/error-while-fetching-from-doi/2018/3)
- We fixed an issue where only the field `abstract` and `comment` were declared as multiline fields. Other fields can now be configured in the preferences using "Do not wrap the following fields when saving" [4373](https://github.com/JabRef/jabref/issues/4373)
- We fixed an issue where JabRef switched to discrete graphics under macOS [#5935](https://github.com/JabRef/jabref/issues/5935)
- We fixed an issue where the Preferences entry preview will be unexpected modified leads to Value too long exception [#6198](https://github.com/JabRef/jabref/issues/6198)
- We fixed an issue where custom jstyles for Open/LibreOffice would only be valid if a layout line for the entry type `default` was at the end of the layout section [#6303](https://github.com/JabRef/jabref/issues/6303)


### Removed

- We removed the obsolete `External programs / Open PDF` section in the preferences, as the default application to open PDFs is now set in the `Manage external file types` dialog. [#6130](https://github.com/JabRef/jabref/pull/6130)
Expand Down
9 changes: 4 additions & 5 deletions src/main/java/org/jabref/gui/fieldeditors/FieldEditors.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

import javax.swing.undo.UndoManager;

import org.jabref.Globals;
import org.jabref.gui.DialogService;
import org.jabref.gui.autocompleter.ContentSelectorSuggestionProvider;
import org.jabref.gui.autocompleter.SuggestionProvider;
Expand Down Expand Up @@ -52,10 +51,10 @@ public static FieldEditorFX getForField(final Field field,
preferences.getBoolean(JabRefPreferences.ENFORCE_LEGAL_BIBTEX_KEY),
preferences.getBoolean(JabRefPreferences.ALLOW_INTEGER_EDITION_BIBTEX));

final boolean isSingleLine = FieldFactory.isSingleLineField(field);
boolean isMultiLine = FieldFactory.isMultiLineField(field, preferences.getFieldContentParserPreferences().getNonWrappableFields());
Copy link
Member

Choose a reason for hiding this comment

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

Can we use an enum here? I really don't like the booleans - and we see changing the flag from singleLine to multiLine was necessary of readability. (Otherweise, the variable would be kept to flag singleLine - and negation would be used)

I propose to use a boolean enum as proposed at https://stackoverflow.com/a/12713550/873282

enum USED_LINES {
    MULTI(true), SINGLE(false);
...

Maybe, just a simple enum would also be OK. Then, the construction from boolean would be a bit more tricky though.

Copy link
Member

Choose a reason for hiding this comment

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

If you want to go in this direction, an enum FieldType with Text and TextLong would make the most sense and conveys the semantics. Can then be extended in the future to include Keywords, Numeric etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

I really would like to leave it for the moment and implement a more generic solution in a follow up PR. I think about integrating it in the custom entry types dialog.


if (preferences.getTimestampPreferences().getTimestampField().equals(field)) {
return new DateEditor(field, DateTimeFormatter.ofPattern(Globals.prefs.getTimestampPreferences().getTimestampFormat()), suggestionProvider, fieldCheckers);
return new DateEditor(field, DateTimeFormatter.ofPattern(preferences.getTimestampPreferences().getTimestampFormat()), suggestionProvider, fieldCheckers);
} else if (fieldProperties.contains(FieldProperty.DATE)) {
return new DateEditor(field, DateTimeFormatter.ofPattern("[uuuu][-MM][-dd]"), suggestionProvider, fieldCheckers);
} else if (fieldProperties.contains(FieldProperty.EXTERNAL)) {
Expand Down Expand Up @@ -89,14 +88,14 @@ public static FieldEditorFX getForField(final Field field,
} else if (fieldProperties.contains(FieldProperty.MULTIPLE_ENTRY_LINK)) {
return new LinkedEntriesEditor(field, databaseContext, suggestionProvider, fieldCheckers);
} else if (fieldProperties.contains(FieldProperty.PERSON_NAMES)) {
return new PersonsEditor(field, suggestionProvider, preferences, fieldCheckers, isSingleLine);
return new PersonsEditor(field, suggestionProvider, preferences, fieldCheckers, isMultiLine);
} else if (StandardField.KEYWORDS.equals(field)) {
return new KeywordsEditor(field, suggestionProvider, fieldCheckers, preferences);
} else if (field == InternalField.KEY_FIELD) {
return new BibtexKeyEditor(field, preferences, suggestionProvider, fieldCheckers, databaseContext, undoManager, dialogService);
} else {
// default
return new SimpleEditor(field, suggestionProvider, fieldCheckers, preferences, isSingleLine);
return new SimpleEditor(field, suggestionProvider, fieldCheckers, preferences, isMultiLine);
}
}

Expand Down
6 changes: 2 additions & 4 deletions src/main/java/org/jabref/gui/fieldeditors/PersonsEditor.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,10 @@ public PersonsEditor(final Field field,
final SuggestionProvider<?> suggestionProvider,
final JabRefPreferences preferences,
final FieldCheckers fieldCheckers,
final boolean isSingleLine) {
final boolean isMultiLine) {
this.viewModel = new PersonsEditorViewModel(field, suggestionProvider, preferences.getAutoCompletePreferences(), fieldCheckers);

textInput = isSingleLine
? new EditorTextField()
: new EditorTextArea();
textInput = isMultiLine ? new EditorTextArea() : new EditorTextField();
Copy link
Member

Choose a reason for hiding this comment

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

Not that the true/false branches were just be wrapped. Another indicator for an enum!


decoratedStringProperty = new UiThreadStringProperty(viewModel.textProperty());
textInput.textProperty().bindBidirectional(decoratedStringProperty);
Expand Down
6 changes: 2 additions & 4 deletions src/main/java/org/jabref/gui/fieldeditors/SimpleEditor.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,10 @@ public SimpleEditor(final Field field,
final SuggestionProvider<?> suggestionProvider,
final FieldCheckers fieldCheckers,
final JabRefPreferences preferences,
final boolean isSingleLine) {
final boolean isMultiLine) {
this.viewModel = new SimpleEditorViewModel(field, suggestionProvider, fieldCheckers);

textInput = isSingleLine
? new EditorTextField()
: new EditorTextArea();
textInput = isMultiLine ? new EditorTextArea() : new EditorTextField();
HBox.setHgrow(textInput, Priority.ALWAYS);

textInput.textProperty().bindBidirectional(viewModel.textProperty());
Expand Down
10 changes: 3 additions & 7 deletions src/main/java/org/jabref/model/entry/field/FieldFactory.java
Original file line number Diff line number Diff line change
Expand Up @@ -139,13 +139,9 @@ public static List<Field> getDefaultGeneralFields() {
return defaultGeneralFields;
}

// TODO: Move somewhere more appropriate and make user-configurable
public static boolean isSingleLineField(final Field field) {
if (field.equals(StandardField.ABSTRACT) || field.equals(StandardField.COMMENT)) {
return false;
}

// TODO: This should ideally be user configurable! Move somewhere more appropriate in the future
public static boolean isMultiLineField(final Field field, List<Field> nonWrappableFields) {
// Treat unknown fields as multi-line fields
return !(field instanceof UnknownField);
return (field instanceof UnknownField) || nonWrappableFields.contains(field) || field.equals(StandardField.ABSTRACT) || field.equals(StandardField.COMMENT) || field.equals(StandardField.REVIEW);
}
}
3 changes: 3 additions & 0 deletions src/main/java/org/jabref/preferences/JabRefPreferences.java
Original file line number Diff line number Diff line change
Expand Up @@ -1465,12 +1465,14 @@ public JournalAbbreviationPreferences getJournalAbbreviationPreferences() {
return new JournalAbbreviationPreferences(getStringList(EXTERNAL_JOURNAL_LISTS), getDefaultEncoding());
}

@Override
public CleanupPreferences getCleanupPreferences(JournalAbbreviationRepository abbreviationRepository) {
return new CleanupPreferences(
getLayoutFormatterPreferences(abbreviationRepository),
getFilePreferences());
}

@Override
public CleanupPreset getCleanupPreset() {
Set<CleanupPreset.CleanupStep> activeJobs = EnumSet.noneOf(CleanupPreset.CleanupStep.class);

Expand All @@ -1485,6 +1487,7 @@ public CleanupPreset getCleanupPreset() {
return new CleanupPreset(activeJobs, formatterCleanups);
}

@Override
public void setCleanupPreset(CleanupPreset cleanupPreset) {
for (CleanupPreset.CleanupStep action : EnumSet.allOf(CleanupPreset.CleanupStep.class)) {
putBoolean(JabRefPreferences.CLEANUP + action.name(), cleanupPreset.isActive(action));
Expand Down