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

Improve Automatic Field Editor Dialog #8973

Merged
merged 81 commits into from
Aug 1, 2022
Merged

Improve Automatic Field Editor Dialog #8973

merged 81 commits into from
Aug 1, 2022

Conversation

HoussemNasri
Copy link
Member

@HoussemNasri HoussemNasri commented Jul 13, 2022

Fixes #8970
Closes #8971
Follow-up to #8892

  • Change in CHANGELOG.md described in a way that is understandable for the average user (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 developer's documentation: Is the information available and up to date? If not, I outlined it in this pull request.
  • Checked documentation: Is the information available and up to date? If not, I created an issue at https://github.com/JabRef/user-documentation/issues or, even better, I submitted a pull request to the documentation repository.

HoussemNasri and others added 27 commits March 24, 2022 01:47
This reverts commit d71f93a.
Why? I argue the word content can be understood more easily than value. The word "value" can be attached to many things e.g. also to field names. The word "content" on the other hand means a value is within something and therefore in this specific context would be more specific.

Co-authored-by: ThiloteE <[email protected]>
@ThiloteE ThiloteE added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Jul 23, 2022
@@ -758,7 +758,7 @@ public static String quoteStringIfSpaceIsContained(String string) {
* @param s The string to check
* @return {@code True} if the given string does contain at least one whitespace character, {@code False} otherwise
* */
public static boolean containsAnyWhitespaceCharacters(String s) {
return s.matches(".*\s.*");
public static boolean containsWhitespace(String s) {
Copy link
Member

Choose a reason for hiding this comment

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

You now need to adapt the failing architecture test to the new size of the StringUtil class.
PS: I wasn't aware of the String.chars method, that's neat

Copy link
Member Author

Choose a reason for hiding this comment

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

Increased it to 765

@calixtus
Copy link
Member

calixtus commented Jul 27, 2022

There were no issues fixed since the last minor release but issues, that were introduced by #8892, so this PR should not have an entry in the changelog.

@calixtus calixtus requested a review from Siedlerchr July 27, 2022 18:22
@Siedlerchr
Copy link
Member

In general this looks good to me so far. I only don't like the EventBus introduction here. It's kind of deprecated and should be avoided if not absolutely necessary.

Why not have an observable count in the ViewModel that you can subscribe to in the tab? We use a similar approach for updating the search results. It's a simple IntegerProperty that is in the stateManager

@HoussemNasri
Copy link
Member Author

I had no idea that using event bus is deprecated. It's actually one of my favourite bits of our architecture. Using an observable count could be a solution, but what if two consecutive actions affect the same number of entries, causing the count to fire only once?

@Siedlerchr
Copy link
Member

I would go with observables. Well, if the same number happens you would need to set it to zero before or so before firing the second one.
Regarding the EventBus, there are several reasons:
https://guava.dev/releases/snapshot-jre/api/docs/com/google/common/eventbus/EventBus.html

@calixtus
Copy link
Member

EventBus is dead. We were planning to remove it for some time.

koppor and others added 3 commits August 1, 2022 20:51
Co-authored-by: Christoph <[email protected]>
Co-authored-by: Carl Christian Snethlage <[email protected]>
Co-authored-by: ThiloteE <[email protected]>
Co-authored-by: Jonatan Asketorp <[email protected]>
Co-authored-by: Christoph <[email protected]>
Co-authored-by: Carl Christian Snethlage <[email protected]>
Co-authored-by: ThiloteE <[email protected]>
Co-authored-by: Jonatan Asketorp <[email protected]>
Co-authored-by: Christoph <[email protected]>
Co-authored-by: ThiloteE <[email protected]>
Co-authored-by: Jonatan Asketorp <[email protected]>
@koppor koppor merged commit 0d82506 into JabRef:main Aug 1, 2022
Siedlerchr added a commit that referenced this pull request Aug 3, 2022
* upstream/main: (21 commits)
  New Crowdin updates (#9016)
  New Crowdin updates (#9013)
  try to gather more output from LO exception (#9002)
  Improve Automatic Field Editor Dialog (#8973)
  Update BST VM to Antlr4 (#8934)
  Support biblatex apa citation for legal entry types (#8966)
  Bump junit-jupiter from 5.8.2 to 5.9.0 (#9012)
  Bump lucene-core from 9.2.0 to 9.3.0 (#9009)
  Bump checkstyle from 10.3.1 to 10.3.2 (#9006)
  Bump appleboy/ssh-action from 0.1.4 to 0.1.5 (#9005)
  New translations JabRef_en.properties (Spanish) (#9003)
  Squashed 'buildres/csl/csl-locales/' changes from 55459cd79f..e637746677
  Squashed 'buildres/csl/csl-styles/' changes from 3d3573c..c750b6e
  Autosave folder and checkbox is remembered (#9000)
  New Crowdin updates (#8999)
  Sync group view mode and main table (#8993)
  Bump unoloader from 7.3.4 to 7.3.5 (#8996)
  Bump libreoffice from 7.3.4 to 7.3.5 (#8997)
  Bump java-diff-utils from 4.11 to 4.12 (#8998)
  Fix external group metadata changes are not merged (#8994)
  ...
Siedlerchr added a commit that referenced this pull request Aug 3, 2022
* upstream/main: (39 commits)
  New Crowdin updates (#9016)
  New Crowdin updates (#9013)
  try to gather more output from LO exception (#9002)
  Improve Automatic Field Editor Dialog (#8973)
  Update BST VM to Antlr4 (#8934)
  Support biblatex apa citation for legal entry types (#8966)
  Bump junit-jupiter from 5.8.2 to 5.9.0 (#9012)
  Bump lucene-core from 9.2.0 to 9.3.0 (#9009)
  Bump checkstyle from 10.3.1 to 10.3.2 (#9006)
  Bump appleboy/ssh-action from 0.1.4 to 0.1.5 (#9005)
  New translations JabRef_en.properties (Spanish) (#9003)
  Squashed 'buildres/csl/csl-locales/' changes from 55459cd79f..e637746677
  Squashed 'buildres/csl/csl-styles/' changes from 3d3573c..c750b6e
  Autosave folder and checkbox is remembered (#9000)
  New Crowdin updates (#8999)
  Sync group view mode and main table (#8993)
  Bump unoloader from 7.3.4 to 7.3.5 (#8996)
  Bump libreoffice from 7.3.4 to 7.3.5 (#8997)
  Bump java-diff-utils from 4.11 to 4.12 (#8998)
  Fix external group metadata changes are not merged (#8994)
  ...
Siedlerchr added a commit to zkl-ai/jabref that referenced this pull request Aug 5, 2022
…failure-dialog

* upstream/main: (31 commits)
  Snapcraft and issue template
  Show development information\n\n+semver: minor
  Release v5.7
  New Crowdin updates (JabRef#9030)
  New Crowdin updates (JabRef#9029)
  [Bot] Update CSL styles (JabRef#9027)
  Add missing translations for AutomaticFieldEditor (JabRef#9028)
  [Bot] Update Journal abbrev list (JabRef#9026)
  Rating in main table (JabRef#9023)
  New Crowdin updates (JabRef#9024)
  New Crowdin updates (JabRef#9016)
  New Crowdin updates (JabRef#9013)
  try to gather more output from LO exception (JabRef#9002)
  Improve Automatic Field Editor Dialog (JabRef#8973)
  Update BST VM to Antlr4 (JabRef#8934)
  Support biblatex apa citation for legal entry types (JabRef#8966)
  Bump junit-jupiter from 5.8.2 to 5.9.0 (JabRef#9012)
  Bump lucene-core from 9.2.0 to 9.3.0 (JabRef#9009)
  Bump checkstyle from 10.3.1 to 10.3.2 (JabRef#9006)
  Bump appleboy/ssh-action from 0.1.4 to 0.1.5 (JabRef#9005)
  ...
Siedlerchr added a commit that referenced this pull request Aug 6, 2022
* upstream/main: (185 commits)
  Snapcraft and issue template
  Show development information\n\n+semver: minor
  Release v5.7
  New Crowdin updates (#9030)
  New Crowdin updates (#9029)
  [Bot] Update CSL styles (#9027)
  Add missing translations for AutomaticFieldEditor (#9028)
  [Bot] Update Journal abbrev list (#9026)
  Rating in main table (#9023)
  New Crowdin updates (#9024)
  New Crowdin updates (#9016)
  New Crowdin updates (#9013)
  try to gather more output from LO exception (#9002)
  Improve Automatic Field Editor Dialog (#8973)
  Update BST VM to Antlr4 (#8934)
  Support biblatex apa citation for legal entry types (#8966)
  Bump junit-jupiter from 5.8.2 to 5.9.0 (#9012)
  Bump lucene-core from 9.2.0 to 9.3.0 (#9009)
  Bump checkstyle from 10.3.1 to 10.3.2 (#9006)
  Bump appleboy/ssh-action from 0.1.4 to 0.1.5 (#9005)
  ...
Siedlerchr added a commit that referenced this pull request Aug 6, 2022
…rg.jsoup-jsoup-1.15.2

* upstream/main: (34 commits)
  Refactor of DOI import failure dialog, import format reader and clipboard manager (#8839)
  Snapcraft and issue template
  Show development information\n\n+semver: minor
  Release v5.7
  New Crowdin updates (#9030)
  New Crowdin updates (#9029)
  [Bot] Update CSL styles (#9027)
  Add missing translations for AutomaticFieldEditor (#9028)
  [Bot] Update Journal abbrev list (#9026)
  Rating in main table (#9023)
  New Crowdin updates (#9024)
  New Crowdin updates (#9016)
  New Crowdin updates (#9013)
  try to gather more output from LO exception (#9002)
  Improve Automatic Field Editor Dialog (#8973)
  Update BST VM to Antlr4 (#8934)
  Support biblatex apa citation for legal entry types (#8966)
  Bump junit-jupiter from 5.8.2 to 5.9.0 (#9012)
  Bump lucene-core from 9.2.0 to 9.3.0 (#9009)
  Bump checkstyle from 10.3.1 to 10.3.2 (#9006)
  ...
@HoussemNasri HoussemNasri deleted the improve-auto-field-editor branch September 13, 2022 06:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automatic-field-editor status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers
Projects
None yet
5 participants