From 3e3ee5ad37c246a049a34f57ba9fe1c9ee3703c8 Mon Sep 17 00:00:00 2001 From: Justin Baumann Date: Tue, 10 Oct 2023 14:47:13 +0200 Subject: [PATCH 1/5] Make update check faster --- sidekick_core/lib/sidekick_core.dart | 64 +++++++++++++++++----------- 1 file changed, 38 insertions(+), 26 deletions(-) diff --git a/sidekick_core/lib/sidekick_core.dart b/sidekick_core/lib/sidekick_core.dart index 8264ecbb..0fac5c13 100644 --- a/sidekick_core/lib/sidekick_core.dart +++ b/sidekick_core/lib/sidekick_core.dart @@ -284,8 +284,20 @@ class SidekickCommandRunner extends CompletionCommandRunner { final unmount = mount(debugName: args.join(' ')); ArgResults? parsedArgs; + Future? updateCheck; + try { parsedArgs = parse(args); + + if (_isUpdateCheckEnabled && + !_isSidekickCliUpdateCommand(parsedArgs) && + !_isRunningTabCompletionCommand(parsedArgs)) { + // print warning if CLI update is available + // TODO prevent multiple update checks when a command start another command + + updateCheck = _checkForUpdates(); + } + if (parsedArgs['version'] == true) { print('${SidekickContext.cliName} is using sidekick version $version'); return null; @@ -294,23 +306,13 @@ class SidekickCommandRunner extends CompletionCommandRunner { final result = await super.runCommand(parsedArgs); return result; } finally { - // don't print anything additionally when running the hidden tab completion command (runs in the background when pressing tab), - // otherwise anything that is printed will also be used as suggestion - bool isRunningTabCompletionCommand() { - final reservedCommands = [ - HandleCompletionRequestCommand.commandName, - InstallCompletionFilesCommand.commandName, - ]; - return reservedCommands.contains(parsedArgs?.command?.name); - } - // don't show the install-global suggesting when running the install-global command bool isInstallGlobalCommand() => parsedArgs?.command?.name == 'sidekick' && parsedArgs!.arguments.contains('install-global'); if (!enableAutoInstall && - !isRunningTabCompletionCommand() && + !_isRunningTabCompletionCommand(parsedArgs) && !isInstallGlobalCommand()) { final command = yellow('./${SidekickContext.cliName} sidekick install-global'); @@ -320,16 +322,7 @@ class SidekickCommandRunner extends CompletionCommandRunner { } try { - if (_isUpdateCheckEnabled && - !_isSidekickCliUpdateCommand(parsedArgs) && - !isRunningTabCompletionCommand()) { - // print warning if the user didn't fully update their CLI - _checkCliVersionIntegrity(); - // print warning if CLI update is available - // TODO start the update check in the background at command start - // TODO prevent multiple update checks when a command start another command - await _checkForUpdates(); - } + (await updateCheck)?.call(); } finally { unmount(); } @@ -337,7 +330,8 @@ class SidekickCommandRunner extends CompletionCommandRunner { } /// Print a warning if the CLI isn't up to date - Future _checkForUpdates() async { + Future _checkForUpdates() async { + void Function()? updateCheck; try { final updateFuture = VersionChecker.isDependencyUpToDate( package: SidekickContext.sidekickPackage, @@ -346,15 +340,21 @@ class SidekickCommandRunner extends CompletionCommandRunner { ); // If it takes too long, don't wait for it final isUpToDate = await updateFuture.timeout(const Duration(seconds: 3)); + if (!isUpToDate) { - printerr( - '${yellow('Update available!')}\n' - 'Run ${cyan('${SidekickContext.cliName} sidekick update')} to update your CLI.', - ); + updateCheck = () => printerr( + '${yellow('Update available!')}\n' + 'Run ${cyan('${SidekickContext.cliName} sidekick update')} to update your CLI.', + ); } } catch (_) { /* ignore */ } + return () { + // print warning if the user didn't fully update their CLI + _checkCliVersionIntegrity(); + updateCheck?.call(); + }; } /// Print a warning if the user manually updated the sidekick_core @@ -397,6 +397,18 @@ class SidekickCommandRunner extends CompletionCommandRunner { } } + /// Returns true if the executed command is a tab completion command + /// + /// don't print anything additionally when running the hidden tab completion command (runs in the background when pressing tab), + /// otherwise anything that is printed will also be used as suggestion + bool _isRunningTabCompletionCommand(ArgResults? parsedArgs) { + final reservedCommands = [ + HandleCompletionRequestCommand.commandName, + InstallCompletionFilesCommand.commandName, + ]; + return reservedCommands.contains(parsedArgs?.command?.name); + } + /// Returns true if the command executed from [parsedArgs] is [UpdateCommand] /// /// Copied and adapted from CommandRunner.runCommand From b9d4e9ab7cd0a9130eb1f4a0e24936522e75db96 Mon Sep 17 00:00:00 2001 From: Justin Baumann Date: Tue, 10 Oct 2023 14:52:35 +0200 Subject: [PATCH 2/5] run checkForUpdates in isolate --- sidekick_core/lib/sidekick_core.dart | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/sidekick_core/lib/sidekick_core.dart b/sidekick_core/lib/sidekick_core.dart index 0fac5c13..57108e4d 100644 --- a/sidekick_core/lib/sidekick_core.dart +++ b/sidekick_core/lib/sidekick_core.dart @@ -1,6 +1,8 @@ /// The core library for Sidekick CLIs library sidekick_core; +import 'dart:isolate'; + import 'package:cli_completion/cli_completion.dart'; import 'package:sidekick_core/sidekick_core.dart'; import 'package:sidekick_core/src/commands/update_command.dart'; @@ -295,7 +297,7 @@ class SidekickCommandRunner extends CompletionCommandRunner { // print warning if CLI update is available // TODO prevent multiple update checks when a command start another command - updateCheck = _checkForUpdates(); + updateCheck = Isolate.run(_checkForUpdates); } if (parsedArgs['version'] == true) { From 0bd0153a75f37711dc24c32d3207700515077176 Mon Sep 17 00:00:00 2001 From: Justin Baumann Date: Tue, 10 Oct 2023 15:04:44 +0200 Subject: [PATCH 3/5] rollback --- sidekick_core/lib/sidekick_core.dart | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sidekick_core/lib/sidekick_core.dart b/sidekick_core/lib/sidekick_core.dart index 57108e4d..0fac5c13 100644 --- a/sidekick_core/lib/sidekick_core.dart +++ b/sidekick_core/lib/sidekick_core.dart @@ -1,8 +1,6 @@ /// The core library for Sidekick CLIs library sidekick_core; -import 'dart:isolate'; - import 'package:cli_completion/cli_completion.dart'; import 'package:sidekick_core/sidekick_core.dart'; import 'package:sidekick_core/src/commands/update_command.dart'; @@ -297,7 +295,7 @@ class SidekickCommandRunner extends CompletionCommandRunner { // print warning if CLI update is available // TODO prevent multiple update checks when a command start another command - updateCheck = Isolate.run(_checkForUpdates); + updateCheck = _checkForUpdates(); } if (parsedArgs['version'] == true) { From bd6e76d081b2f4c409c1efd6bcaa2aa59f4df5fb Mon Sep 17 00:00:00 2001 From: Justin Baumann Date: Sat, 4 Nov 2023 15:15:46 +0100 Subject: [PATCH 4/5] simplify --- sidekick_core/lib/sidekick_core.dart | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/sidekick_core/lib/sidekick_core.dart b/sidekick_core/lib/sidekick_core.dart index 0fac5c13..732d28b9 100644 --- a/sidekick_core/lib/sidekick_core.dart +++ b/sidekick_core/lib/sidekick_core.dart @@ -284,7 +284,7 @@ class SidekickCommandRunner extends CompletionCommandRunner { final unmount = mount(debugName: args.join(' ')); ArgResults? parsedArgs; - Future? updateCheck; + Future? updateCheck; try { parsedArgs = parse(args); @@ -322,7 +322,11 @@ class SidekickCommandRunner extends CompletionCommandRunner { } try { - (await updateCheck)?.call(); + // print warning if the user didn't fully update their CLI + _checkCliVersionIntegrity(); + await updateCheck?.then((result) { + if (result != null) printerr(result); + }); } finally { unmount(); } @@ -330,8 +334,7 @@ class SidekickCommandRunner extends CompletionCommandRunner { } /// Print a warning if the CLI isn't up to date - Future _checkForUpdates() async { - void Function()? updateCheck; + Future _checkForUpdates() async { try { final updateFuture = VersionChecker.isDependencyUpToDate( package: SidekickContext.sidekickPackage, @@ -342,19 +345,13 @@ class SidekickCommandRunner extends CompletionCommandRunner { final isUpToDate = await updateFuture.timeout(const Duration(seconds: 3)); if (!isUpToDate) { - updateCheck = () => printerr( - '${yellow('Update available!')}\n' - 'Run ${cyan('${SidekickContext.cliName} sidekick update')} to update your CLI.', - ); + return '${yellow('Update available!')}\n' + 'Run ${cyan('${SidekickContext.cliName} sidekick update')} to update your CLI.'; } } catch (_) { /* ignore */ } - return () { - // print warning if the user didn't fully update their CLI - _checkCliVersionIntegrity(); - updateCheck?.call(); - }; + return null; } /// Print a warning if the user manually updated the sidekick_core From 42c41c8db0d8ebc51c3600c9b58ff45c6a609151 Mon Sep 17 00:00:00 2001 From: Justin Baumann Date: Sat, 4 Nov 2023 15:26:53 +0100 Subject: [PATCH 5/5] fix tests --- sidekick_core/lib/sidekick_core.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sidekick_core/lib/sidekick_core.dart b/sidekick_core/lib/sidekick_core.dart index 732d28b9..dae10700 100644 --- a/sidekick_core/lib/sidekick_core.dart +++ b/sidekick_core/lib/sidekick_core.dart @@ -322,9 +322,9 @@ class SidekickCommandRunner extends CompletionCommandRunner { } try { - // print warning if the user didn't fully update their CLI - _checkCliVersionIntegrity(); await updateCheck?.then((result) { + // print warning if the user didn't fully update their CLI + _checkCliVersionIntegrity(); if (result != null) printerr(result); }); } finally {