Skip to content

Process CLI arguments before starting gui#9217

Merged
tobiasdiez merged 6 commits into
mainfrom
launcher_impro
Oct 12, 2022
Merged

Process CLI arguments before starting gui#9217
tobiasdiez merged 6 commits into
mainfrom
launcher_impro

Conversation

@tobiasdiez

@tobiasdiez tobiasdiez commented Oct 5, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

This moves the whole parsing of CLI args and basic initalization to the very beginning, even before the JavaFX toolkit is even started. In this way, the response time for the CLI is improved very much, which is particularily important for the browser extension.
I wanted to do this for a very long time, but it was not possible since the preferences pulled-in too much of the GUI. But with the latest refactors, this is no longer the case 🎉

Also:

  • Demote some of the log calls to debug
  • Remove unused test class
  • 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.

@tobiasdiez tobiasdiez added the status: ready-for-review Pull Requests that are ready to be reviewed by the maintainers label Oct 5, 2022
github-actions[bot]
github-actions Bot reviewed Oct 5, 2022
View reviewed changes
Comment thread src/main/java/org/jabref/gui/MainApplication.java Outdated
Comment thread src/main/java/org/jabref/gui/MainApplication.java
Comment thread src/main/java/org/jabref/gui/MainApplication.java
Comment thread src/main/java/org/jabref/gui/MainApplication.java
Comment thread src/main/java/org/jabref/cli/Launcher.java
Comment thread src/main/java/org/jabref/cli/Launcher.java Outdated
Comment thread src/main/java/org/jabref/cli/Launcher.java Outdated
Siedlerchr
Siedlerchr reviewed Oct 6, 2022
View reviewed changes
Comment thread src/main/java/org/jabref/gui/desktop/JabRefDesktop.java Outdated
Siedlerchr
Siedlerchr reviewed Oct 6, 2022
View reviewed changes
Comment thread src/main/java/org/jabref/gui/logging/GuiWriter.java
@calixtus

calixtus commented Oct 6, 2022

Copy link
Copy Markdown
Member

Yeah, coding tobi is back.

calixtus
calixtus reviewed Oct 6, 2022
View reviewed changes
Comment thread src/main/java/org/jabref/cli/Launcher.java
calixtus
calixtus reviewed Oct 6, 2022
View reviewed changes
Comment thread src/main/java/org/jabref/gui/MainApplication.java
@calixtus

calixtus commented Oct 6, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

There seems to be a design flaw in my eyes: The ArgumentsProcessor reads and loads some databases on startup before running the gui and hands them already opened and loaded to the GUI or the CLI. But this has nothing to do with this PR, so should be adressed in another PR.

@tobiasdiez tobiasdiez mentioned this pull request Oct 7, 2022
Merged
6 tasks
@tobiasdiez

tobiasdiez commented Oct 7, 2022

Copy link
Copy Markdown
Member Author

There seems to be a design flaw in my eyes: The ArgumentsProcessor reads and loads some databases on startup before running the gui and hands them already opened and loaded to the GUI or the CLI. But this has nothing to do with this PR, so should be adressed in another PR.

Good point, this would improve the opening of files by double-clicking a bit (or at least the user would get an earlier visual feedback). But that's something for a next PR.

@koppor

koppor commented Oct 7, 2022

Copy link
Copy Markdown
Member

In the long run, we should work on #110 - to be able to offer a cli tool and a separate GUI. This requires supporting the work by @calixtus to make the preferences more nice.

@tobiasdiez

tobiasdiez commented Oct 10, 2022

Copy link
Copy Markdown
Member Author

I've now moved the controversial default file directory business to #9222. The rest is cleanup up based on previous feedback (thanks!) and should be good to go.

calixtus
calixtus reviewed Oct 10, 2022
View reviewed changes

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

Looks good to me

@tobiasdiez

tobiasdiez commented Oct 12, 2022

Copy link
Copy Markdown
Member Author

Thanks for the review! Failing test are not due to this PR, so merging now.

@tobiasdiez tobiasdiez merged commit c827c43 into main Oct 12, 2022
@tobiasdiez tobiasdiez deleted the launcher_impro branch October 12, 2022 12:25
@Siedlerchr Siedlerchr mentioned this pull request Oct 12, 2022
Closed
@koppor

koppor commented Oct 13, 2022

Copy link
Copy Markdown
Member

We should have two reviewers - especially by architecturally significant changes!

Siedlerchr added a commit that referenced this pull request Oct 13, 2022
@Siedlerchr
Merge remote-tracking branch 'upstream/main' into restapi
607cb2a 
* upstream/main: (42 commits)
  Place subgroups w.r.t. alphabetical ordering (#9228)
  DOAB Fetcher now fetches ISBN and imprint fields where possible (#9229)
  ISSUE-9145: implement isbn fetcher (#9205)
  Remove explicit javafx jmod stuff (#9245)
  New Crowdin updates (#9239)
  Support Arabic language (#9243)
  Process CLI arguments before starting gui (#9217)
  Update README Decision Records link (#9241)
  Opening a file removes duplicate keywords and triggers lib changed (#9216)
  New Crowdin updates (#9237)
  Add Ukrainian as language (#9236)
  Add ADR-0026 on using Swing's FileChooser (#9235)
  Log exception (#9227)
  Fix archunit (#9234)
  Bump tika-core from 2.4.1 to 2.5.0 (#9232)
  Check group type before showing dialog in edit group (#8909)
  Fix unexpected closing of preferences dialog (#9225)
  Fix typo in connection error message (#9226)
  Improve VSCode config (#9218)
  Searching for an entry on the web shouldn't take forever (#9199)
  ...

# Conflicts:
#	build.gradle
#	lib/afterburner.fx.jar
#	src/main/java/org/jabref/cli/Launcher.java
#	src/main/java/org/jabref/gui/EntryTypeView.java
#	src/main/java/org/jabref/gui/auximport/FromAuxDialog.java
#	src/main/java/org/jabref/gui/bibtexextractor/ExtractBibtexDialog.java
#	src/main/java/org/jabref/gui/collab/ExternalChangesResolverDialog.java
#	src/main/java/org/jabref/gui/commonfxcontrols/CitationKeyPatternPanel.java
#	src/main/java/org/jabref/gui/contentselector/ContentSelectorDialogView.java
#	src/main/java/org/jabref/gui/customentrytypes/CustomizeEntryTypeDialogView.java
#	src/main/java/org/jabref/gui/documentviewer/DocumentViewerView.java
#	src/main/java/org/jabref/gui/edit/ManageKeywordsDialog.java
#	src/main/java/org/jabref/gui/entryeditor/EntryEditor.java
#	src/main/java/org/jabref/gui/entryeditor/fileannotationtab/FileAnnotationTabView.java
#	src/main/java/org/jabref/gui/errorconsole/ErrorConsoleView.java
#	src/main/java/org/jabref/gui/exporter/CreateModifyExporterDialogView.java
#	src/main/java/org/jabref/gui/externalfiles/UnlinkedFilesDialogView.java
#	src/main/java/org/jabref/gui/help/AboutDialogView.java
#	src/main/java/org/jabref/gui/importer/ImportCustomEntryTypesDialog.java
#	src/main/java/org/jabref/gui/importer/ImportEntriesDialog.java
#	src/main/java/org/jabref/gui/integrity/IntegrityCheckDialog.java
#	src/main/java/org/jabref/gui/libraryproperties/AbstractPropertiesTabView.java
#	src/main/java/org/jabref/gui/libraryproperties/LibraryPropertiesView.java
#	src/main/java/org/jabref/gui/libraryproperties/constants/ConstantsPropertiesView.java
#	src/main/java/org/jabref/gui/libraryproperties/general/GeneralPropertiesView.java
#	src/main/java/org/jabref/gui/libraryproperties/keypattern/KeyPatternPropertiesView.java
#	src/main/java/org/jabref/gui/libraryproperties/saving/SavingPropertiesView.java
#	src/main/java/org/jabref/gui/linkedfile/LinkedFileEditDialogView.java
#	src/main/java/org/jabref/gui/openoffice/ManageCitationsDialogView.java
#	src/main/java/org/jabref/gui/openoffice/StyleSelectDialogView.java
#	src/main/java/org/jabref/gui/preferences/AbstractPreferenceTabView.java
#	src/main/java/org/jabref/gui/preferences/PreferencesDialogView.java
#	src/main/java/org/jabref/gui/preferences/customexporter/CustomExporterTab.java
#	src/main/java/org/jabref/gui/preferences/externalfiletypes/EditExternalFileTypeEntryDialog.java
#	src/main/java/org/jabref/gui/preferences/journals/JournalAbbreviationsTab.java
#	src/main/java/org/jabref/gui/preferences/keybindings/KeyBindingsTab.java
#	src/main/java/org/jabref/gui/preferences/preview/PreviewTab.java
#	src/main/java/org/jabref/gui/preferences/protectedterms/ProtectedTermsTab.java
#	src/main/java/org/jabref/gui/search/GlobalSearchResultDialog.java
#	src/main/java/org/jabref/gui/shared/SharedDatabaseLoginDialogView.java
#	src/main/java/org/jabref/gui/slr/ManageStudyDefinitionView.java
#	src/main/java/org/jabref/gui/texparser/ParseLatexDialogView.java
#	src/main/java/org/jabref/gui/texparser/ParseLatexResultView.java
Siedlerchr added a commit that referenced this pull request Oct 17, 2022
@Siedlerchr
Revert "Process CLI arguments before starting gui (#9217)"
bf3b87d 
This reverts commit c827c43.
This was referenced Oct 17, 2022
Closed
Closed
@thiagocferr thiagocferr mentioned this pull request Oct 24, 2022
Closed
2 tasks
@koppor koppor mentioned this pull request May 2, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Siedlerchr Siedlerchr Siedlerchr left review comments

@github-actions github-actions[bot] github-actions[bot] left review comments

@calixtus calixtus calixtus left review comments

Assignees

No one assigned

Labels

component: JabKit [cli] 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.

5 participants

@tobiasdiez @calixtus @koppor @Siedlerchr @ThiloteE