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.

Sign up for GitHub

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

Jump to bottom

Make wrap fields also visible in entry editor #6315

Merged
merged 6 commits into from
May 8, 2020
Merged

Make wrap fields also visible in entry editor #6315

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0b39116
Make wrap fields also visible in entry editor
Siedlerchr Apr 19, 2020
4dab523
Merge remote-tracking branch 'upstream/master' into multiline
Siedlerchr Apr 24, 2020
7d21e06
change comment
Siedlerchr Apr 24, 2020
1dc4fc5
Merge remote-tracking branch 'upstream/master' into multiline
Siedlerchr Apr 27, 2020
3fdb701
add changelog
Siedlerchr Apr 27, 2020
e99a550
Merge branch 'master' into multiline
Siedlerchr May 2, 2020
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
9 changes: 4 additions & 5 deletions src/main/java/org/jabref/gui/fieldeditors/FieldEditors.java
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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) {
Siedlerchr marked this conversation as resolved.
Show resolved Hide resolved
// 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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1538,12 +1538,14 @@ public JournalAbbreviationPreferences getJournalAbbreviationPreferences() {
getBoolean(USE_IEEE_ABRV), getDefaultEncoding());
}

@Override
public CleanupPreferences getCleanupPreferences(JournalAbbreviationLoader journalAbbreviationLoader) {
return new CleanupPreferences(
getLayoutFormatterPreferences(journalAbbreviationLoader),
getFilePreferences());
}

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

Expand All @@ -1558,6 +1560,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