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

Process CLI arguments before starting gui #9217

Merged
merged 6 commits into from
Oct 12, 2022
Merged

Process CLI arguments before starting gui #9217

merged 6 commits into from
Oct 12, 2022

Conversation

tobiasdiez
Copy link
Member

@tobiasdiez tobiasdiez commented Oct 5, 2022 •
edited
Loading

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
@calixtus
Copy link
Member

calixtus commented Oct 6, 2022

Yeah, coding tobi is back.

@calixtus
Copy link
Member

calixtus commented Oct 6, 2022 •
edited
Loading

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
Copy link
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
Copy link
Member

koppor commented Oct 7, 2022

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.

@ThiloteE ThiloteE added the cli label Oct 9, 2022
@tobiasdiez
Copy link
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
Copy link
Member

@calixtus calixtus left a comment

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
Copy link
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
Copy link
Member

koppor commented Oct 13, 2022

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