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

Sorting custom entry fields that contain numerical values #6426

Merged
merged 9 commits into from
May 8, 2020
Merged

Sorting custom entry fields that contain numerical values #6426

merged 9 commits into from
May 8, 2020

Conversation

dimitra-karadima
Copy link
Contributor

@dimitra-karadima dimitra-karadima commented May 5, 2020 •
edited by Siedlerchr
Loading

@MootezSaaD and I have worked together and this pull request fixes #6349
We added a new Comparator for numeric cases and now they are sorted as integers and not alphabetically.

  • Change in CHANGELOG.md described (if applicable)
  • Tests created for changes (if applicable)
  • Manually tested changed features in running JabRef (always required)
  • Screenshots added in PR description (for UI changes)
  • Checked documentation: Is the information available and up to date? If not created an issue at https://github.com/JabRef/user-documentation/issues or, even better, submitted a pull request to the documentation repository.

dimitra-karadima and others added 6 commits May 5, 2020 16:57
@dimitra-karadima
Add new Comparator class
dd44e42 
Add a comparator for numeric cases in order to sort custom entry fields that contain numerical values
@dimitra-karadima
Change Comparator class
7d0d9ff 
Change in order for the Comparator class to wrap String and not Optional<SpecialFieldValueViewModel>
@dimitra-karadima
Merge master into branch fix-for-issue-6349
ce3cc93
@MootezSaaD
Add comparison for numeric values in custom entries
7eaf7d2
@MootezSaaD
Fix issues related to NPE in the comparator
28fe8e3
@dimitra-karadima
Update CHANGELOG.md
3b07d48 
Add the fix we implement in the appropriate section.
Siedlerchr
Siedlerchr requested changes May 6, 2020
View reviewed changes
Copy link
Member

@Siedlerchr Siedlerchr left a comment

Choose a reason for hiding this comment

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

Thanks, looks good so far. It would be really nice you could add a Unit Test for this class.
You can easily test that by using the comparator on a List. with Comparator.comparing(....)

@Siedlerchr Siedlerchr added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label May 6, 2020
@calixtus calixtus changed the base branch from master to dimitra-6349 May 8, 2020 21:03
@calixtus calixtus merged commit 5be42b3 into JabRef:dimitra-6349 May 8, 2020
@Siedlerchr
Copy link
Member

I think something went wrong with the merge?

@calixtus
Copy link
Member

calixtus commented May 8, 2020 •
edited
Loading

Hi @dimitra-karadima ,
@koppor and me had a mob session tonight and we took the liberty to use your PR to teach me a bit about git, since there were only some checkstyle-nitpicks with your PR left. We hope you don't mind we finished your work in #6450 .
Thank you very much for your work on this. We hope you did enjoy working with JabRef. You are always welcome to contribute more.

@koppor koppor mentioned this pull request May 8, 2020
Merged
@dimitra-karadima
Copy link
Contributor Author

dimitra-karadima commented May 9, 2020 •
edited
Loading

Hi @dimitra-karadima ,
@koppor and me had a mob session tonight and we took the liberty to use your PR to teach me a bit about git, since there were only some checkstyle-nitpicks with your PR left. We hope you don't mind we finished your work in #6450 .
Thank you very much for your work on this. We hope you did enjoy working with JabRef. You are always welcome to contribute more.

@calixtus Of course no problem! Thanks for fixing and merging my code!

Siedlerchr added a commit that referenced this pull request May 9, 2020
@Siedlerchr
Merge remote-tracking branch 'upstream/master'
41ebd96 
* upstream/master:
  Ignore generated .mv file
  Fix checkstyle, moved some comments for better understanding
  Sorting custom entry fields that contain numerical values (#6426)
  UI consistency - BibTexStringEditorDialog rework (#6287)
  Squashed 'src/main/resources/csl-styles/' changes from 270cd32..c35d219
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Siedlerchr Siedlerchr Siedlerchr approved these changes

Assignees
No one assigned
Labels
status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Sorting custom entry fields that contain numerical values
4 participants
@dimitra-karadima @Siedlerchr @calixtus @MootezSaaD