search

koppor / jabref

Collection of simple for JabRef issues. Please submit PRs to https://github.com/jabRef/jabref/.
https://github.com/jabRef/jabref/
MIT License
8 stars 13 forks source link

Fix FieldComparator vs. EntryComparator #497

Open koppor opened 3 years ago

koppor commented 3 years ago

The EntryComparator does not call the FieldComparator.

Why?

Should be fixed. - At least JavaDoc should added referencing each other and explaining the differences.

Refs https://github.com/JabRef/jabref/pull/7708

k3KAW8Pnf7mkmdSMPHz27 commented 3 years ago

I'll view this as a bit of a mid-length project. Preliminarily I am looking at

  1. BibtexStringComparator
  2. EntryComparator (break ties with Id)
  3. FieldComparator
  4. FieldComparatorStack (doesn't break ties with Id) -> replace with Comparator.andThen or .reduce(Comparator::thenComparing) -> removed
  5. CrossRefEntryComparator
  6. NumericFieldComparator -> might make a change to its logic, will add test cases if I do
  7. IdComparator
  8. RankingFieldComparator
  9. SpecialFieldComparator
  10. StringLengthComparator -> two uses with Comparator.comparingInt(String::length).reversed()) -> removed

and it seems that EntryComparator and FieldComparatorStack serve the same purpose.

Most of them are essentially unchanged in the last 8 years (i.e., they are pre-GitHub) so I'll take a look at modernizing the ones I believe need it. The changes should lead to a lot less code that is more readable (❤️ Java 8+) so I'd guess it is a one PR change.

Todo

  1. Verify that I can use reduce
k3KAW8Pnf7mkmdSMPHz27 commented 3 years ago

Should the PR go here or in the JabRef/jabref?

koppor commented 1 year ago

I'd guess it is a one PR change.

I fully agree.

Therefore I woud open the PR against JabRef/jabref. Here, we collect PRs, which are suspended or take a veeeeeery long time to finish. This issue here is more a short-term one.