Add ability to run arbitrary formatters as cleanup actions

[tobiasdiez]

Add ability to run arbitrary formatters as cleanup actions by reusing save actions. Thus some old cleanup entries could be removed. This also implements/fixes #295. Moreover, an erase formatter is added which just deletes the corresponding field.

Code changes:

• Rename SaveActions to FieldFormatterCleanups
• Some smaller refactorings to reuse SaveActions (mainly to separate them from MetaData)

:

• Change in CHANGELOG.md described?
• Changes in pull request outlined? (What, why, ...)
• Tests created for changes?
• Tests green?

[lenhard]

I think it would look better if the check boxes were aligned vertically.

[tobiasdiez]

You are right!

[simonharrer]

When I compare both UIs (before/after) this PR, we loose a lot of documentation and guidance for the user. Because how, the user has to think which formatters would be well suited for which fields to be helpful, making it harder to configure it correctly. We need to find a way to fight this. Why not add a section about suggestions which can be checked/unchecked and upon change change the table with the actions as well?

[tobiasdiez]

All the previous cleanup actions appear as the default value for the field formatters:

activeFormatterCleanups.add(new FieldFormatterCleanup("date", BibtexFieldFormatters.DATE));
activeFormatterCleanups.add(new FieldFormatterCleanup("month", new MonthFormatter()));
activeFormatterCleanups.add(new FieldFormatterCleanup("title", new CaseKeeper()));
activeFormatterCleanups.add(new FieldFormatterCleanup("title", new UnitFormatter()));
activeFormatterCleanups.add(new FieldFormatterCleanup("title", new LatexFormatter()));
activeFormatterCleanups.add(new FieldFormatterCleanup("title", new HTMLToLatexFormatter()));

So opening the cleanup dialog for the first time shows essentially the same information as previously (this is not apparent from the screenshots since I already removed some of the default values).

I could add a button which resets the field formatters to their default value (or if you prefer adds the default formatters).

[simonharrer]

I do not like the EraseFormatter. I think this should not be seen as a formatter as it clearly violates the interface. Maybe use something like a BibEntryFormatter?

[tobiasdiez]

You are right, clearing a field is not really a Formatter. I could introduce an interface SingleFieldCleanup which the FormatterCleanup and RemoveFieldCleanup can implement. This new interface can then be used by the SaveActions. Does this sounds like a good solution?

[simonharrer]

Hm, what happens when a field is empty? Is it removed? I could live with an EraseFormatter if it would always return "". Does this make sense?

I do not have all the classes and their reponsibilities in mind for both the formatter and the cleanup. If you need more feedback, you will have to wait as I have to look this stuff up.

[oscargus]

Not really following the discussion, but it struck me that a functionality that removes all empty fields, i.e., where the field is "" (or even whitespace) could be useful...

[lenhard]

I aggree with @oscargus That might be useful as a cleanup action.

[tobiasdiez]

I now changed the eraseFormatter to always returns an empty string. Thus it now meets the Formatter contract.

The RemoveEmptyFieldsCleanup is a good suggestion, which is now tracked at #911.

[simonharrer]

• Make paths of linked files relative if possible could also be a Formatter, and hence, added to the save actions.
• A button which resets to the defaults is a good idea.
• We really need to think how the formatters can be better explained to the user. Would it make sense if each formatter itself has a short localized description how it works, similar to the sentences used before which are shown in the table as well?

[tobiasdiez]

New update with:

• Reset button (in cleanup dialog as well as in database properties dialog)
• Add descriptions of the formatters as tooltips in the list and in the combo box (not ideal, but better then nothing).
  

Adding the "Make paths relative" cleanup as a save action should be covered with #878. I don't want to implement this cleanup as a formatter (formatters are just in-out converters without any side-effects).

[koppor]

• keep name concept. Key is for serialization. Name is for display. Description is a long text.
• improve keys and names of formatters - separate issue Table of formatters #920 with a table: key and name
  • e.g., unit as key
  • e.g., Name: DateFormatter -> NormalizeDateToYYYYMMDD
• Add description text as box below the line of "Enter field name" updated upon change of dropdown
  • e.g., "Normalizes dates ..."

[tobiasdiez]

Added now a description text box. From my side this PR can be merged in. The only missing thing is the naming of the formatters, but I would prefer if this would be a separate PR (not sure how much time I have the next days and since this is a blocker for 3.3....)

[simonharrer]

Last point: what happens when the description is longer than this line? Does it wrap automatically? Does it allow to use newlines for formatting?

[tobiasdiez]

Since this is just an JLabel, all the nice limitations and/or features come along. So no, there are no automatic line breaks except if you put everything around tags and you also need to use <br> for a new line.

[simonharrer]

Hm, this limits the description we can display there. Nevertheless, I think it is best to merge it for now and then do the improvements in another PR. Just rebase and get this in.

[koppor]

I really like it and only have minor issues, which I also can file as separate issue if it is too much effort to resolve now.

Is it possible to display a description with "the given field" in place of the field name if no field is given? Reason: I find the empty description irritating and thought that the content of the white box was the description.

Above at "Save sort order", one can select a field. I would have expected the same choice for the selection of the fields to apply the save action. Reason: I am too lazy to input the field name manually.

I experience layout issues at the end - The dialog height should be the size of all content including the white box, shouldn't it?

[tobiasdiez]

I added your suggestion about adding "the given field". Could you please open an issue for the other things? Thanks!
I'll now merge this in.