Skip to content

Enable triggering offline parsing explicitely#11565

Merged
koppor merged 16 commits into
mainfrom
grobid-preferences
Aug 4, 2024
Merged

Enable triggering offline parsing explicitely#11565
koppor merged 16 commits into
mainfrom
grobid-preferences

Conversation

@koppor

@koppor koppor commented Aug 1, 2024

Copy link
Copy Markdown
Member

New online/offline distinction more visible in the UI:

Library menu:

image

Button on toolbar still asks for permission:

image

If "Do not ask again" (and Yes)

image

Additionally:

Feature: clipboard contents now contained automatically:

image

(You also see here "online" and "offline" indication at the dialog title)


Feature itself is described at https://docs.jabref.org/collect/newentryfromplaintext.

Mandatory checks

  • 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.

koppor added 2 commits August 1, 2024 21:01
- Refactor actions to inheritance instead of if-statements in a class
- Solved "fun" with inheritance and FXML
- Use %0 and offline/offline parameterization
- Rename "GROBID" to "Grobid"
Comment thread build.gradle Outdated
}

repositories {
mavenLocal()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

remove

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I needed mavenLocal() to be able to load the patched version org.jabref:afterburner.fx:2.0.0-SNAPSHOT.

Not required any more.

Comment thread src/main/java/org/jabref/gui/actions/StandardActions.java Outdated
Comment thread src/main/java/org/jabref/gui/bibtexextractor/BibtexExtractorViewModel.java Outdated
Comment thread src/main/java/org/jabref/gui/bibtexextractor/ExtractBibtexDialog.java Outdated

@Siedlerchr Siedlerchr left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

bad architectural changes

@koppor

koppor commented Aug 2, 2024

Copy link
Copy Markdown
Member Author

Just for reference: Code with inheritance side-by-side:

image

I will now try to use composition. Let's see, how it will work out.

@koppor

koppor commented Aug 2, 2024

Copy link
Copy Markdown
Member Author

Rewritten, now we have an if/else in BibtexExtractorViewModel:

    public void startParsing() {
        if (onlineMode) {
            startParsingOnline();
        } else {
            startParsingOffline();
        }
    }

This is OK for me.


Platform.runLater(() -> {
input.requestFocus();
Button buttonParse = (Button) getDialogPane().lookupButton(parseButtonType);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I need to have this code here, because otherwise, I get

Caused by: java.lang.NullPointerException: Cannot invoke "javafx.scene.control.Button.setTooltip(javafx.scene.control.Tooltip)" because "buttonParse" is null

# Conflicts:
#	src/main/java/org/jabref/gui/actions/StandardActions.java
#	src/main/resources/l10n/JabRef_en.properties
public ExtractBibtexDialog(boolean onlineMode) {
this.onlineMode = onlineMode;
ViewLoader.view(this)
.controller(this)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why controller?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh, I thought, I could not it get to work without this... Strange. Removed.

@koppor koppor added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Aug 2, 2024
@koppor koppor enabled auto-merge August 2, 2024 22:13
Siedlerchr
Siedlerchr previously approved these changes Aug 4, 2024
@koppor koppor added this pull request to the merge queue Aug 4, 2024
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 4, 2024
@calixtus calixtus added this pull request to the merge queue Aug 4, 2024
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 4, 2024
@koppor koppor added this pull request to the merge queue Aug 4, 2024
@github-merge-queue github-merge-queue Bot removed this pull request from the merge queue due to failed status checks Aug 4, 2024

@calixtus calixtus left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

NewLibraryFromPdfActionOffline / NewLibraryFromPdfActionOnline : Composition over inheritance.
@koppor promised to fix this in follow up pr.

@koppor koppor added this pull request to the merge queue Aug 4, 2024
@github-actions

github-actions Bot commented Aug 4, 2024

Copy link
Copy Markdown
Contributor

The build for this PR is no longer available. Please visit https://builds.jabref.org/main/ for the latest build.

Merged via the queue into main with commit dc2e17a Aug 4, 2024
@koppor koppor deleted the grobid-preferences branch August 4, 2024 17:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants