From 12eb748299c5ed1e27cdcfda622bfdaead3b4614 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Wed, 10 May 2023 17:30:43 +0100 Subject: [PATCH 01/29] refactor: added ScriptConfigurationEntry --- .../installer/completion_installation.dart | 21 ++- lib/src/installer/installer.dart | 4 + .../installer/script_configuration_entry.dart | 51 +++++++ .../script_configuration_entry_test.dart | 127 ++++++++++++++++++ 4 files changed, 190 insertions(+), 13 deletions(-) create mode 100644 lib/src/installer/installer.dart create mode 100644 lib/src/installer/script_configuration_entry.dart create mode 100644 test/src/installer/script_configuration_entry_test.dart diff --git a/lib/src/installer/completion_installation.dart b/lib/src/installer/completion_installation.dart index 5f6064f..10ddbf2 100644 --- a/lib/src/installer/completion_installation.dart +++ b/lib/src/installer/completion_installation.dart @@ -1,6 +1,7 @@ import 'dart:io'; import 'package:cli_completion/installer.dart'; +import 'package:cli_completion/src/installer/script_configuration_entry.dart'; import 'package:mason_logger/mason_logger.dart'; import 'package:meta/meta.dart'; import 'package:path/path.dart' as path; @@ -207,18 +208,15 @@ class CompletionInstallation { logger.info('No file found at $configPath, creating one now'); configFile.createSync(); } - final commandScriptName = '$rootCommand.${configuration.name}'; - - final containsLine = - configFile.readAsStringSync().contains(commandScriptName); - if (containsLine) { + if (ScriptConfigurationEntry(rootCommand).existsIn(configFile)) { logger.warn( 'A config entry for $rootCommand was already found on $configPath.', ); return; } + final commandScriptName = '$rootCommand.${configuration.name}'; _sourceScriptOnFile( configFile: configFile, scriptName: rootCommand, @@ -307,15 +305,12 @@ class CompletionInstallation { description ??= 'Completion config for "$scriptName"'; - configFile.writeAsStringSync( - mode: FileMode.append, - ''' -\n## [$scriptName] + final content = ''' ## $description -${configuration!.sourceLineTemplate(scriptPath)} -## [/$scriptName] - -''', +${configuration!.sourceLineTemplate(scriptPath)}'''; + ScriptConfigurationEntry(scriptName).appendTo( + configFile, + content: content, ); logger.info('Added config to $configFilePath'); diff --git a/lib/src/installer/installer.dart b/lib/src/installer/installer.dart new file mode 100644 index 0000000..03776bd --- /dev/null +++ b/lib/src/installer/installer.dart @@ -0,0 +1,4 @@ +export 'completion_installation.dart'; +export 'exceptions.dart'; +export 'script_configuration_entry.dart'; +export 'shell_completion_configuration.dart'; diff --git a/lib/src/installer/script_configuration_entry.dart b/lib/src/installer/script_configuration_entry.dart new file mode 100644 index 0000000..e5408f0 --- /dev/null +++ b/lib/src/installer/script_configuration_entry.dart @@ -0,0 +1,51 @@ +import 'dart:io'; + +/// {@template script_entry} +/// A script entry is a section of a file that starts with [_startComment] and +/// ends with [_endComment]. +/// {@endtemplate} +class ScriptConfigurationEntry { + /// {@macro script_entry} + const ScriptConfigurationEntry(this.name) + : _startComment = '\n## [$name] ', + _endComment = '\n## [/$name]\n'; + + /// The name of the entry. + final String name; + + /// The start comment of the entry. + final String _startComment; + + /// The end comment of the entry. + final String _endComment; + + /// Whether there is an entry with [name] in [file]. + /// + /// If the [file] does not exist, this will return false. + bool existsIn(File file) { + if (!file.existsSync()) return false; + final content = file.readAsStringSync(); + return content.contains(_startComment) && content.contains(_endComment); + } + + /// Adds an entry with [name] to the end of the [file]. + /// + /// If the [file] does not exist, it will be created. + /// + /// If [content] is not null, it will be added within the entry. + void appendTo(File file, {String? content}) { + if (!file.existsSync()) { + file.createSync(recursive: true); + } + + final entry = StringBuffer() + ..writeln(_startComment) + ..write(content) + ..writeln(_endComment); + + file.writeAsStringSync( + entry.toString(), + mode: FileMode.append, + ); + } +} diff --git a/test/src/installer/script_configuration_entry_test.dart b/test/src/installer/script_configuration_entry_test.dart new file mode 100644 index 0000000..87103a1 --- /dev/null +++ b/test/src/installer/script_configuration_entry_test.dart @@ -0,0 +1,127 @@ +// ignore_for_file: prefer_const_constructors + +import 'dart:io'; + +import 'package:cli_completion/src/installer/script_configuration_entry.dart'; +import 'package:path/path.dart' as path; +import 'package:test/test.dart'; + +void main() { + group('$ScriptConfigurationEntry', () { + test('can be instatiated', () { + expect(() => ScriptConfigurationEntry('name'), returnsNormally); + }); + + test('has a name', () { + expect(ScriptConfigurationEntry('name').name, 'name'); + }); + + group('appendsTo', () { + test('returns normally when file exist', () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final filePath = path.join(tempDirectory.path, 'file'); + final file = File(filePath)..createSync(); + + final entry = ScriptConfigurationEntry('name'); + + expect( + () => entry.appendTo(file), + returnsNormally, + ); + }); + + test('returns normally when file does not exist', () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final filePath = path.join(tempDirectory.path, 'file'); + final file = File(filePath); + + final entry = ScriptConfigurationEntry('name'); + + expect( + () => entry.appendTo(file), + returnsNormally, + ); + }); + + test('correctly appends content', () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final filePath = path.join(tempDirectory.path, 'file'); + final file = File(filePath)..createSync(); + const initialContent = 'hello world\n'; + file.writeAsStringSync(initialContent); + + final entry = ScriptConfigurationEntry('name'); + const entryContent = 'hello world'; + + entry.appendTo(file, content: entryContent); + + final fileContent = file.readAsStringSync(); + const expectedContent = ''' +$initialContent +## [name] +$entryContent +## [/name]\n +'''; + expect(fileContent, equals(expectedContent)); + }); + }); + + group('existsIn', () { + group('returns false', () { + test('when when file does not exist', () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final filePath = path.join(tempDirectory.path, 'file'); + final file = File(filePath); + + final entry = ScriptConfigurationEntry('name'); + + expect(entry.existsIn(file), isFalse); + }); + + test('when file exists without entry', () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final filePath = path.join(tempDirectory.path, 'file'); + final file = File(filePath)..createSync(); + + final entry = ScriptConfigurationEntry('name'); + + expect(entry.existsIn(file), isFalse); + }); + + test('when file exists with another entry', () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final filePath = path.join(tempDirectory.path, 'file'); + final file = File(filePath)..createSync(); + ScriptConfigurationEntry('other').appendTo(file); + + final entry = ScriptConfigurationEntry('name'); + expect(entry.existsIn(file), isFalse); + }); + }); + + test('returns true when file exists with an entry', () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final filePath = path.join(tempDirectory.path, 'file'); + final file = File(filePath)..createSync(); + + final entry = ScriptConfigurationEntry('name')..appendTo(file); + + expect(entry.existsIn(file), isTrue); + }); + }); + }); +} From 000aa81b61d43e7c91c01beebcf13c929459d5d3 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Wed, 10 May 2023 17:35:20 +0100 Subject: [PATCH 02/29] refactor: remove empty space --- lib/src/installer/script_configuration_entry.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/installer/script_configuration_entry.dart b/lib/src/installer/script_configuration_entry.dart index e5408f0..b28acfc 100644 --- a/lib/src/installer/script_configuration_entry.dart +++ b/lib/src/installer/script_configuration_entry.dart @@ -7,7 +7,7 @@ import 'dart:io'; class ScriptConfigurationEntry { /// {@macro script_entry} const ScriptConfigurationEntry(this.name) - : _startComment = '\n## [$name] ', + : _startComment = '\n## [$name]', _endComment = '\n## [/$name]\n'; /// The name of the entry. From ac0a310c0837b185899299ffb5fd11afdb60c489 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Wed, 10 May 2023 17:35:32 +0100 Subject: [PATCH 03/29] refactor: use completion installation to verify installation --- lib/src/installer/completion_installation.dart | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/src/installer/completion_installation.dart b/lib/src/installer/completion_installation.dart index 10ddbf2..5e39732 100644 --- a/lib/src/installer/completion_installation.dart +++ b/lib/src/installer/completion_installation.dart @@ -254,12 +254,10 @@ class CompletionInstallation { ); } - final containsLine = - shellRCFile.readAsStringSync().contains(completionConfigPath); - - if (containsLine) { - logger.warn('A completion config entry was already found on' - ' $_shellRCFilePath.'); + if (const ScriptConfigurationEntry('Completion').existsIn(shellRCFile)) { + logger.warn( + '''A completion config entry was already found on $_shellRCFilePath.''', + ); return; } From 4421964a8a9f682a118256ed86964a260a9da4ce Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Wed, 10 May 2023 17:38:21 +0100 Subject: [PATCH 04/29] refactor: removing unused code --- lib/src/installer/completion_installation.dart | 8 -------- 1 file changed, 8 deletions(-) diff --git a/lib/src/installer/completion_installation.dart b/lib/src/installer/completion_installation.dart index 5e39732..f235f3e 100644 --- a/lib/src/installer/completion_installation.dart +++ b/lib/src/installer/completion_installation.dart @@ -238,15 +238,7 @@ class CompletionInstallation { 'to $_shellRCFilePath', ); - final completionConfigDirPath = completionConfigDir.path; - - final completionConfigPath = path.join( - completionConfigDirPath, - configuration.completionConfigForShellFileName, - ); - final shellRCFile = File(_shellRCFilePath); - if (!shellRCFile.existsSync()) { throw CompletionInstallationException( rootCommand: rootCommand, From 8e6939ce33a603cdeefd1e4f3030eeba761ed5e1 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Wed, 10 May 2023 17:38:30 +0100 Subject: [PATCH 05/29] feat: reformatted start and end comments --- lib/src/installer/script_configuration_entry.dart | 10 ++++++---- .../src/installer/script_configuration_entry_test.dart | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lib/src/installer/script_configuration_entry.dart b/lib/src/installer/script_configuration_entry.dart index b28acfc..ff4b60d 100644 --- a/lib/src/installer/script_configuration_entry.dart +++ b/lib/src/installer/script_configuration_entry.dart @@ -7,8 +7,8 @@ import 'dart:io'; class ScriptConfigurationEntry { /// {@macro script_entry} const ScriptConfigurationEntry(this.name) - : _startComment = '\n## [$name]', - _endComment = '\n## [/$name]\n'; + : _startComment = '## [$name]', + _endComment = '## [/$name]'; /// The name of the entry. final String name; @@ -39,9 +39,11 @@ class ScriptConfigurationEntry { } final entry = StringBuffer() + ..writeln() ..writeln(_startComment) - ..write(content) - ..writeln(_endComment); + ..writeln(content) + ..writeln(_endComment) + ..writeln(); file.writeAsStringSync( entry.toString(), diff --git a/test/src/installer/script_configuration_entry_test.dart b/test/src/installer/script_configuration_entry_test.dart index 87103a1..8df8b9f 100644 --- a/test/src/installer/script_configuration_entry_test.dart +++ b/test/src/installer/script_configuration_entry_test.dart @@ -64,7 +64,7 @@ void main() { final fileContent = file.readAsStringSync(); const expectedContent = ''' $initialContent -## [name] +## [name] $entryContent ## [/name]\n '''; From 3397c163c251f5c6408504e7d18758b6f4ab3077 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Wed, 10 May 2023 17:39:15 +0100 Subject: [PATCH 06/29] refactor: used barrel file --- lib/installer.dart | 4 +--- lib/src/installer/completion_installation.dart | 1 - 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/lib/installer.dart b/lib/installer.dart index fe5ee55..d34b03a 100644 --- a/lib/installer.dart +++ b/lib/installer.dart @@ -2,7 +2,5 @@ /// {@canonicalFor system_shell.SystemShell} library installer; -export 'src/installer/completion_installation.dart'; -export 'src/installer/exceptions.dart'; -export 'src/installer/shell_completion_configuration.dart'; +export 'src/installer/installer.dart'; export 'src/system_shell.dart'; diff --git a/lib/src/installer/completion_installation.dart b/lib/src/installer/completion_installation.dart index f235f3e..b2c6f41 100644 --- a/lib/src/installer/completion_installation.dart +++ b/lib/src/installer/completion_installation.dart @@ -1,7 +1,6 @@ import 'dart:io'; import 'package:cli_completion/installer.dart'; -import 'package:cli_completion/src/installer/script_configuration_entry.dart'; import 'package:mason_logger/mason_logger.dart'; import 'package:meta/meta.dart'; import 'package:path/path.dart' as path; From 6d37050c8780443ec788fbffcf9b918af7e19142 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Wed, 10 May 2023 17:40:10 +0100 Subject: [PATCH 07/29] test: refactor expectation --- test/src/installer/script_configuration_entry_test.dart | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/test/src/installer/script_configuration_entry_test.dart b/test/src/installer/script_configuration_entry_test.dart index 8df8b9f..7b0a70c 100644 --- a/test/src/installer/script_configuration_entry_test.dart +++ b/test/src/installer/script_configuration_entry_test.dart @@ -66,7 +66,8 @@ void main() { $initialContent ## [name] $entryContent -## [/name]\n +## [/name] + '''; expect(fileContent, equals(expectedContent)); }); From c9cca0a3e9f787a11774d582906bb0f3fc0f16fe Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Thu, 11 May 2023 18:45:30 +0100 Subject: [PATCH 08/29] test: remove white space --- test/src/installer/completion_installation_test.dart | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/src/installer/completion_installation_test.dart b/test/src/installer/completion_installation_test.dart index c57307c..97f49e9 100644 --- a/test/src/installer/completion_installation_test.dart +++ b/test/src/installer/completion_installation_test.dart @@ -286,7 +286,7 @@ void main() { // ignore: leading_newlines_in_multiline_strings expect(rcFile.readAsStringSync(), ''' -\n## [Completion] +\n## [Completion] ## Completion scripts setup. Remove the following line to uninstall [[ -f ${configDir.path}/zsh-config.zsh ]] && . ${configDir.path}/zsh-config.zsh || true ## [/Completion] @@ -394,7 +394,7 @@ void main() { // ignore: leading_newlines_in_multiline_strings expect(rcFile.readAsStringSync(), ''' -\n## [Completion] +\n## [Completion] ## Completion scripts setup. Remove the following line to uninstall [[ -f ${configDir.path}/zsh-config.zsh ]] && . ${configDir.path}/zsh-config.zsh || true ## [/Completion] @@ -408,12 +408,12 @@ void main() { // ignore: leading_newlines_in_multiline_strings expect(globalConfig.readAsStringSync(), ''' -\n## [very_good] +\n## [very_good] ## Completion config for "very_good" [[ -f ${configDir.path}/very_good.zsh ]] && . ${configDir.path}/very_good.zsh || true ## [/very_good] -\n## [not_good] +\n## [not_good] ## Completion config for "not_good" [[ -f ${configDir.path}/not_good.zsh ]] && . ${configDir.path}/not_good.zsh || true ## [/not_good] @@ -447,7 +447,7 @@ void main() { // ignore: leading_newlines_in_multiline_strings expect(bashProfile.readAsStringSync(), ''' -\n## [Completion] +\n## [Completion] ## Completion scripts setup. Remove the following line to uninstall [ -f ${configDir.path}/bash-config.bash ] && . ${configDir.path}/bash-config.bash || true ## [/Completion] From ac810c368f0c9759eb339007ba4eca6082b74b35 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Thu, 11 May 2023 18:54:50 +0100 Subject: [PATCH 09/29] feat: fixed merge issue --- lib/src/installer/completion_installation.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/installer/completion_installation.dart b/lib/src/installer/completion_installation.dart index bb5b597..732951d 100644 --- a/lib/src/installer/completion_installation.dart +++ b/lib/src/installer/completion_installation.dart @@ -215,7 +215,7 @@ class CompletionInstallation { return; } - final commandScriptName = '$rootCommand.${configuration.name}'; + final commandScriptName = '$rootCommand.${configuration.shell.name}'; _sourceScriptOnFile( configFile: configFile, scriptName: rootCommand, From 98f4cf0f1a53535a1f2a6d30f03a0b17c9aa47af Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Thu, 11 May 2023 18:57:00 +0100 Subject: [PATCH 10/29] test: removed extra space --- test/src/installer/completion_installation_test.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/src/installer/completion_installation_test.dart b/test/src/installer/completion_installation_test.dart index 38853e3..421dc63 100644 --- a/test/src/installer/completion_installation_test.dart +++ b/test/src/installer/completion_installation_test.dart @@ -225,7 +225,7 @@ void main() { // ignore: leading_newlines_in_multiline_strings expect(configFile.readAsStringSync(), ''' -\n## [very_good] +\n## [very_good] ## Completion config for "very_good" [[ -f ${configDir.path}/very_good.zsh ]] && . ${configDir.path}/very_good.zsh || true ## [/very_good] From ccf8de322fcc0576a4e6e49c61f95eb864c7974d Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Thu, 11 May 2023 20:27:31 +0100 Subject: [PATCH 11/29] feat: uninstall logic --- .../installer/completion_installation.dart | 59 +++++++++++++++++++ lib/src/installer/exceptions.dart | 21 +++++++ .../installer/script_configuration_entry.dart | 29 +++++++++ 3 files changed, 109 insertions(+) diff --git a/lib/src/installer/completion_installation.dart b/lib/src/installer/completion_installation.dart index 732951d..c768563 100644 --- a/lib/src/installer/completion_installation.dart +++ b/lib/src/installer/completion_installation.dart @@ -304,6 +304,65 @@ ${configuration!.sourceLineTemplate(scriptPath)}'''; logger.info('Added config to $configFilePath'); } + + /// Uninstalls the completion for the command [executableName] on the current + /// shell. + /// + /// Before uninstalling, it checks if the completion is installed: + /// - The shell has an existing RCFile with a completion + /// [ScriptConfigurationEntry]. + /// - The shell has an exisiting configuration config file with a + /// [ScriptConfigurationEntry] for the [executableName]. + /// + /// If any of the above is not true, it throws a + /// [CompletionUnistallationException]. + /// + /// Upon a successful uninstallation the executable [ScriptConfigurationEntry] + /// is removed from the shell config file. If after this removal the latter is + /// empty, it is deleted. In addition, the executable completion script is + /// deleted if it exists. + void uninstall(String executableName) { + final configuration = this.configuration!; + logger.detail( + '''Uninstalling completion for the command $executableName on ${configuration.shell.name}''', + ); + + final shellRCFile = File(_shellRCFilePath); + if (!const ScriptConfigurationEntry('Completion').existsIn(shellRCFile)) { + throw CompletionUnistallationException( + executableName: executableName, + message: 'No configuration file found at ${shellRCFile.path}', + ); + } + + final shellCompletionConfigurationFile = File( + path.join( + completionConfigDir.path, + configuration.completionConfigForShellFileName, + ), + ); + final executableEntry = ScriptConfigurationEntry(executableName); + if (!executableEntry.existsIn(shellCompletionConfigurationFile)) { + throw CompletionUnistallationException( + executableName: executableName, + message: + 'No script file found at ${shellCompletionConfigurationFile.path}', + ); + } + + final executableShellCompletionScriptFile = File( + path.join( + completionConfigDir.path, + '$executableName.${configuration.shell.name}', + ), + ); + if (executableShellCompletionScriptFile.existsSync()) { + executableShellCompletionScriptFile.deleteSync(); + } + + executableEntry.removeFrom(shellCompletionConfigurationFile); + // TODO(alestiago): Add .uninstall file to avoid autocompletion installing again. + } } /// Resolve the home from a path string diff --git a/lib/src/installer/exceptions.dart b/lib/src/installer/exceptions.dart index 06106b1..d7bc58a 100644 --- a/lib/src/installer/exceptions.dart +++ b/lib/src/installer/exceptions.dart @@ -18,3 +18,24 @@ class CompletionInstallationException implements Exception { String toString() => 'Could not install completion scripts for $rootCommand: ' '$message'; } + +/// {@template completion_unistallation_exception} +/// Describes an exception during the installation of completion scripts. +/// {@endtemplate} +class CompletionUnistallationException implements Exception { + /// {@macro completion_unistallation_exception} + CompletionUnistallationException({ + required this.message, + required this.executableName, + }); + + /// The error message for this exception + final String message; + + /// The command for which the installation failed. + final String executableName; + + @override + String toString() => + '''Could not uninstall completion scripts for $executableName: $message'''; +} diff --git a/lib/src/installer/script_configuration_entry.dart b/lib/src/installer/script_configuration_entry.dart index ff4b60d..f035666 100644 --- a/lib/src/installer/script_configuration_entry.dart +++ b/lib/src/installer/script_configuration_entry.dart @@ -50,4 +50,33 @@ class ScriptConfigurationEntry { mode: FileMode.append, ); } + + /// Removes the entry with [name] from the [file]. + /// + /// If the [file] does not exist, this will do nothing. + /// + /// If a file has multiple entries with the same [name], all of them will be + /// removed. + /// + /// If the [file] is empty after removing the entry, it will be deleted. + void removeFrom(File file) { + if (!file.existsSync()) return; + + final content = file.readAsStringSync(); + + int entryStart; + int entryEnd; + do { + entryStart = content.indexOf(_startComment); + entryEnd = content.indexOf(_endComment) + _endComment.length; + final entry = content.substring(entryStart, entryEnd); + file.writeAsStringSync( + content.replaceFirst(entry, ''), + ); + } while (entryStart != -1 && entryEnd != -1); + + if (content.isEmpty) { + file.deleteSync(); + } + } } From fd16cfab50b05bf9cb2a49aca5ddb22daa082a98 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Fri, 12 May 2023 21:04:19 +0100 Subject: [PATCH 12/29] test: removeFrom --- .../script_configuration_entry_test.dart | 97 +++++++++++++++++++ 1 file changed, 97 insertions(+) diff --git a/test/src/installer/script_configuration_entry_test.dart b/test/src/installer/script_configuration_entry_test.dart index 7b0a70c..fe0238c 100644 --- a/test/src/installer/script_configuration_entry_test.dart +++ b/test/src/installer/script_configuration_entry_test.dart @@ -124,5 +124,102 @@ $entryContent expect(entry.existsIn(file), isTrue); }); }); + + group('removeFrom', () { + test('returns normally when file does not exist', () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final filePath = path.join(tempDirectory.path, 'file'); + final file = File(filePath); + + final entry = ScriptConfigurationEntry('name'); + + expect(() => entry.removeFrom(file), returnsNormally); + }); + + test('does not change the file when file is empty', () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final filePath = path.join(tempDirectory.path, 'file'); + final file = File(filePath)..createSync(); + + ScriptConfigurationEntry('name').removeFrom(file); + + final content = file.readAsStringSync(); + expect(content, isEmpty); + }); + + test('does not change the file when another entry exists in file', () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final filePath = path.join(tempDirectory.path, 'file'); + final file = File(filePath)..createSync(); + + ScriptConfigurationEntry('name').appendTo(file); + final content = file.readAsStringSync(); + + ScriptConfigurationEntry('anotherName').removeFrom(file); + + final newContent = file.readAsStringSync(); + expect(content, equals(newContent)); + }); + + test('removes entry from file when there is a single matching entry', () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final filePath = path.join(tempDirectory.path, 'file'); + final file = File(filePath)..createSync(); + + final entry = ScriptConfigurationEntry('name')..appendTo(file); + expect(entry.existsIn(file), isTrue); + + ScriptConfigurationEntry('name').removeFrom(file); + final content = file.readAsStringSync(); + expect(content, isEmpty); + }); + + test( + '''removes all entries from file when there is a multiple matching entries''', + () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final filePath = path.join(tempDirectory.path, 'file'); + final file = File(filePath)..createSync(); + + final entry = ScriptConfigurationEntry('name') + ..appendTo(file) + ..appendTo(file) + ..appendTo(file); + expect(entry.existsIn(file), isTrue); + + ScriptConfigurationEntry('name').removeFrom(file); + final content = file.readAsStringSync(); + expect(content, isEmpty); + }); + + test('only removes matching entries from file', () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final filePath = path.join(tempDirectory.path, 'file'); + final file = File(filePath)..createSync(); + + final entry = ScriptConfigurationEntry('name')..appendTo(file); + expect(entry.existsIn(file), isTrue); + + final anotherEntry = ScriptConfigurationEntry('anotherName') + ..appendTo(file); + expect(anotherEntry.existsIn(file), isTrue); + + ScriptConfigurationEntry('name').removeFrom(file); + final content = file.readAsStringSync(); + expect(content, isEmpty); + }); + }); }); } From 2b19a14c0da82ca407dafb048ab551607e6a9bf4 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Fri, 12 May 2023 21:04:55 +0100 Subject: [PATCH 13/29] fix: added break condition --- lib/src/installer/script_configuration_entry.dart | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/src/installer/script_configuration_entry.dart b/lib/src/installer/script_configuration_entry.dart index f035666..2407441 100644 --- a/lib/src/installer/script_configuration_entry.dart +++ b/lib/src/installer/script_configuration_entry.dart @@ -69,6 +69,7 @@ class ScriptConfigurationEntry { do { entryStart = content.indexOf(_startComment); entryEnd = content.indexOf(_endComment) + _endComment.length; + if (entryStart == -1 || entryEnd == -1) break; final entry = content.substring(entryStart, entryEnd); file.writeAsStringSync( content.replaceFirst(entry, ''), From 817ae1bd4c1be376e0fcdadf428917178633b439 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Fri, 12 May 2023 21:06:38 +0100 Subject: [PATCH 14/29] refactor: used while instead of do while --- lib/src/installer/script_configuration_entry.dart | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/lib/src/installer/script_configuration_entry.dart b/lib/src/installer/script_configuration_entry.dart index 2407441..94ee231 100644 --- a/lib/src/installer/script_configuration_entry.dart +++ b/lib/src/installer/script_configuration_entry.dart @@ -64,17 +64,16 @@ class ScriptConfigurationEntry { final content = file.readAsStringSync(); - int entryStart; - int entryEnd; - do { - entryStart = content.indexOf(_startComment); - entryEnd = content.indexOf(_endComment) + _endComment.length; - if (entryStart == -1 || entryEnd == -1) break; + var entryStart = content.indexOf(_startComment); + var entryEnd = content.indexOf(_endComment) + _endComment.length; + while (entryStart != -1 && entryEnd != -1) { final entry = content.substring(entryStart, entryEnd); file.writeAsStringSync( content.replaceFirst(entry, ''), ); - } while (entryStart != -1 && entryEnd != -1); + entryStart = content.indexOf(_startComment); + entryEnd = content.indexOf(_endComment) + _endComment.length; + } if (content.isEmpty) { file.deleteSync(); From c87eb667a5b73e2f94d49376240870bd211af700 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Sat, 13 May 2023 13:08:47 +0100 Subject: [PATCH 15/29] feat: refined removedFrom --- .../installer/script_configuration_entry.dart | 32 +++++------ .../script_configuration_entry_test.dart | 53 ++++++++++++------- 2 files changed, 50 insertions(+), 35 deletions(-) diff --git a/lib/src/installer/script_configuration_entry.dart b/lib/src/installer/script_configuration_entry.dart index 94ee231..1eb37bd 100644 --- a/lib/src/installer/script_configuration_entry.dart +++ b/lib/src/installer/script_configuration_entry.dart @@ -38,15 +38,16 @@ class ScriptConfigurationEntry { file.createSync(recursive: true); } - final entry = StringBuffer() + final stringBuffer = StringBuffer() ..writeln() - ..writeln(_startComment) - ..writeln(content) + ..writeln(_startComment); + if (content != null) stringBuffer.writeln(content); + stringBuffer ..writeln(_endComment) ..writeln(); file.writeAsStringSync( - entry.toString(), + stringBuffer.toString(), mode: FileMode.append, ); } @@ -63,19 +64,18 @@ class ScriptConfigurationEntry { if (!file.existsSync()) return; final content = file.readAsStringSync(); + final stringPattern = '\n$_startComment.*$_endComment\n\n' + .replaceAll('[', r'\[') + .replaceAll(']', r'\]'); + final pattern = RegExp( + stringPattern, + multiLine: true, + dotAll: true, + ); + final newContent = content.replaceAllMapped(pattern, (_) => ''); + file.writeAsStringSync(newContent); - var entryStart = content.indexOf(_startComment); - var entryEnd = content.indexOf(_endComment) + _endComment.length; - while (entryStart != -1 && entryEnd != -1) { - final entry = content.substring(entryStart, entryEnd); - file.writeAsStringSync( - content.replaceFirst(entry, ''), - ); - entryStart = content.indexOf(_startComment); - entryEnd = content.indexOf(_endComment) + _endComment.length; - } - - if (content.isEmpty) { + if (newContent.trim().isEmpty) { file.deleteSync(); } } diff --git a/test/src/installer/script_configuration_entry_test.dart b/test/src/installer/script_configuration_entry_test.dart index fe0238c..c6e6d62 100644 --- a/test/src/installer/script_configuration_entry_test.dart +++ b/test/src/installer/script_configuration_entry_test.dart @@ -56,10 +56,8 @@ void main() { const initialContent = 'hello world\n'; file.writeAsStringSync(initialContent); - final entry = ScriptConfigurationEntry('name'); const entryContent = 'hello world'; - - entry.appendTo(file, content: entryContent); + ScriptConfigurationEntry('name').appendTo(file, content: entryContent); final fileContent = file.readAsStringSync(); const expectedContent = ''' @@ -68,6 +66,27 @@ $initialContent $entryContent ## [/name] +'''; + expect(fileContent, equals(expectedContent)); + }); + + test('correctly appends content when null', () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final filePath = path.join(tempDirectory.path, 'file'); + final file = File(filePath)..createSync(); + const initialContent = 'hello world\n'; + file.writeAsStringSync(initialContent); + + ScriptConfigurationEntry('name').appendTo(file); + + final fileContent = file.readAsStringSync(); + const expectedContent = ''' +$initialContent +## [name] +## [/name] + '''; expect(fileContent, equals(expectedContent)); }); @@ -138,7 +157,7 @@ $entryContent expect(() => entry.removeFrom(file), returnsNormally); }); - test('does not change the file when file is empty', () { + test('deletes file when file is empty', () { final tempDirectory = Directory.systemTemp.createTempSync(); addTearDown(() => tempDirectory.deleteSync(recursive: true)); @@ -147,8 +166,7 @@ $entryContent ScriptConfigurationEntry('name').removeFrom(file); - final content = file.readAsStringSync(); - expect(content, isEmpty); + expect(file.existsSync(), isFalse); }); test('does not change the file when another entry exists in file', () { @@ -163,11 +181,11 @@ $entryContent ScriptConfigurationEntry('anotherName').removeFrom(file); - final newContent = file.readAsStringSync(); - expect(content, equals(newContent)); + final currentContent = file.readAsStringSync(); + expect(content, equals(currentContent)); }); - test('removes entry from file when there is a single matching entry', () { + test('removes file when there is a single matching entry', () { final tempDirectory = Directory.systemTemp.createTempSync(); addTearDown(() => tempDirectory.deleteSync(recursive: true)); @@ -178,13 +196,10 @@ $entryContent expect(entry.existsIn(file), isTrue); ScriptConfigurationEntry('name').removeFrom(file); - final content = file.readAsStringSync(); - expect(content, isEmpty); + expect(file.existsSync(), isFalse); }); - test( - '''removes all entries from file when there is a multiple matching entries''', - () { + test('''removes file when there is a multiple matching entries''', () { final tempDirectory = Directory.systemTemp.createTempSync(); addTearDown(() => tempDirectory.deleteSync(recursive: true)); @@ -198,8 +213,7 @@ $entryContent expect(entry.existsIn(file), isTrue); ScriptConfigurationEntry('name').removeFrom(file); - final content = file.readAsStringSync(); - expect(content, isEmpty); + expect(file.existsSync(), isFalse); }); test('only removes matching entries from file', () { @@ -211,14 +225,15 @@ $entryContent final entry = ScriptConfigurationEntry('name')..appendTo(file); expect(entry.existsIn(file), isTrue); + final newContent = file.readAsStringSync(); final anotherEntry = ScriptConfigurationEntry('anotherName') ..appendTo(file); expect(anotherEntry.existsIn(file), isTrue); - ScriptConfigurationEntry('name').removeFrom(file); - final content = file.readAsStringSync(); - expect(content, isEmpty); + ScriptConfigurationEntry('anotherName').removeFrom(file); + final actualContent = file.readAsStringSync(); + expect(actualContent, equals(newContent)); }); }); }); From b571c146e11bce0e41c0468543a97dab845e8faf Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Sat, 13 May 2023 13:19:54 +0100 Subject: [PATCH 16/29] feat: refined uninstall logic --- lib/src/installer/completion_installation.dart | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/lib/src/installer/completion_installation.dart b/lib/src/installer/completion_installation.dart index c768563..a1baaae 100644 --- a/lib/src/installer/completion_installation.dart +++ b/lib/src/installer/completion_installation.dart @@ -319,8 +319,8 @@ ${configuration!.sourceLineTemplate(scriptPath)}'''; /// /// Upon a successful uninstallation the executable [ScriptConfigurationEntry] /// is removed from the shell config file. If after this removal the latter is - /// empty, it is deleted. In addition, the executable completion script is - /// deleted if it exists. + /// empty, it is deleted together with the the executable completion script + /// and the completion [ScriptConfigurationEntry] from the shell RC file. void uninstall(String executableName) { final configuration = this.configuration!; logger.detail( @@ -328,7 +328,8 @@ ${configuration!.sourceLineTemplate(scriptPath)}'''; ); final shellRCFile = File(_shellRCFilePath); - if (!const ScriptConfigurationEntry('Completion').existsIn(shellRCFile)) { + const completionEntry = ScriptConfigurationEntry('Completion'); + if (!completionEntry.existsIn(shellRCFile)) { throw CompletionUnistallationException( executableName: executableName, message: 'No configuration file found at ${shellRCFile.path}', @@ -350,6 +351,11 @@ ${configuration!.sourceLineTemplate(scriptPath)}'''; ); } + executableEntry.removeFrom(shellCompletionConfigurationFile); + if (!shellCompletionConfigurationFile.existsSync()) { + completionEntry.removeFrom(shellRCFile); + } + final executableShellCompletionScriptFile = File( path.join( completionConfigDir.path, @@ -360,7 +366,6 @@ ${configuration!.sourceLineTemplate(scriptPath)}'''; executableShellCompletionScriptFile.deleteSync(); } - executableEntry.removeFrom(shellCompletionConfigurationFile); // TODO(alestiago): Add .uninstall file to avoid autocompletion installing again. } } From e8335c90f41ddb4780716c2f219809a021689a85 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Sat, 13 May 2023 13:57:08 +0100 Subject: [PATCH 17/29] feat: tested and configured uninstalling behaviour --- .../installer/completion_installation.dart | 15 +- .../installer/script_configuration_entry.dart | 7 +- .../completion_installation_test.dart | 209 ++++++++++++++++++ .../script_configuration_entry_test.dart | 20 +- 4 files changed, 244 insertions(+), 7 deletions(-) diff --git a/lib/src/installer/completion_installation.dart b/lib/src/installer/completion_installation.dart index a1baaae..d2ac205 100644 --- a/lib/src/installer/completion_installation.dart +++ b/lib/src/installer/completion_installation.dart @@ -328,11 +328,18 @@ ${configuration!.sourceLineTemplate(scriptPath)}'''; ); final shellRCFile = File(_shellRCFilePath); + if (!shellRCFile.existsSync()) { + throw CompletionUnistallationException( + executableName: executableName, + message: 'No shell RC file found at ${shellRCFile.path}', + ); + } + const completionEntry = ScriptConfigurationEntry('Completion'); if (!completionEntry.existsIn(shellRCFile)) { throw CompletionUnistallationException( executableName: executableName, - message: 'No configuration file found at ${shellRCFile.path}', + message: 'Completion is not installed at ${shellRCFile.path}', ); } @@ -347,13 +354,15 @@ ${configuration!.sourceLineTemplate(scriptPath)}'''; throw CompletionUnistallationException( executableName: executableName, message: - 'No script file found at ${shellCompletionConfigurationFile.path}', + '''No shell script file found at ${shellCompletionConfigurationFile.path}''', ); } executableEntry.removeFrom(shellCompletionConfigurationFile); if (!shellCompletionConfigurationFile.existsSync()) { - completionEntry.removeFrom(shellRCFile); + // TODO(alestiago): Should we check if there are other shells installed, before + // deleting the completion config dir? + completionEntry.removeFrom(shellRCFile, shouldDelete: false); } final executableShellCompletionScriptFile = File( diff --git a/lib/src/installer/script_configuration_entry.dart b/lib/src/installer/script_configuration_entry.dart index 1eb37bd..1f94447 100644 --- a/lib/src/installer/script_configuration_entry.dart +++ b/lib/src/installer/script_configuration_entry.dart @@ -59,8 +59,9 @@ class ScriptConfigurationEntry { /// If a file has multiple entries with the same [name], all of them will be /// removed. /// - /// If the [file] is empty after removing the entry, it will be deleted. - void removeFrom(File file) { + /// If [shouldDelete] is true, the [file] will be deleted if it is empty after + /// removing the entry. Otherwise, the [file] will be left empty. + void removeFrom(File file, {bool shouldDelete = true}) { if (!file.existsSync()) return; final content = file.readAsStringSync(); @@ -75,7 +76,7 @@ class ScriptConfigurationEntry { final newContent = content.replaceAllMapped(pattern, (_) => ''); file.writeAsStringSync(newContent); - if (newContent.trim().isEmpty) { + if (shouldDelete && newContent.trim().isEmpty) { file.deleteSync(); } } diff --git a/test/src/installer/completion_installation_test.dart b/test/src/installer/completion_installation_test.dart index 421dc63..fa5289b 100644 --- a/test/src/installer/completion_installation_test.dart +++ b/test/src/installer/completion_installation_test.dart @@ -493,5 +493,214 @@ void main() { }, ); }); + + group('uninstall', () { + test( + '''deletes entire completion configuration when there is a single executable''', + () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final configuration = zshConfiguration; + final installation = CompletionInstallation( + logger: logger, + isWindows: false, + environment: { + 'HOME': tempDirectory.path, + }, + configuration: configuration, + ); + + final rcFile = File(path.join(tempDirectory.path, '.zshrc')) + ..createSync(); + const ScriptConfigurationEntry('Completion').appendTo(rcFile); + + final shellCompletionConfigurationFile = File( + path.join( + installation.completionConfigDir.path, + configuration.completionConfigForShellFileName, + ), + ); + const executableName = 'very_good'; + const ScriptConfigurationEntry(executableName) + .appendTo(shellCompletionConfigurationFile); + + final executableCompletionConfigurationFile = File( + path.join( + installation.completionConfigDir.path, + '$executableName.zsh', + ), + )..createSync(); + + installation.uninstall(executableName); + + expect(rcFile.existsSync(), isTrue); + expect(rcFile.readAsStringSync(), isEmpty); + expect(shellCompletionConfigurationFile.existsSync(), false); + expect(executableCompletionConfigurationFile.existsSync(), false); + }); + + test( + '''deletes executable completion configuration when there are multiple installed executables''', + () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final configuration = zshConfiguration; + final installation = CompletionInstallation( + logger: logger, + isWindows: false, + environment: { + 'HOME': tempDirectory.path, + }, + configuration: configuration, + ); + + final rcFile = File(path.join(tempDirectory.path, '.zshrc')) + ..createSync(); + final completionEntry = const ScriptConfigurationEntry('Completion') + ..appendTo(rcFile); + + final shellCompletionConfigurationFile = File( + path.join( + installation.completionConfigDir.path, + configuration.completionConfigForShellFileName, + ), + ); + const executableName = 'very_good'; + final executableEntry = const ScriptConfigurationEntry(executableName) + ..appendTo(shellCompletionConfigurationFile); + final executableCompletionConfigurationFile = File( + path.join( + installation.completionConfigDir.path, + '$executableName.zsh', + ), + )..createSync(); + + const anotherExecutableName = 'not_good'; + final anotherExecutableEntry = + const ScriptConfigurationEntry(anotherExecutableName) + ..appendTo(shellCompletionConfigurationFile); + final anotherExecutableCompletionConfigurationFile = File( + path.join( + installation.completionConfigDir.path, + '$anotherExecutableName.zsh', + ), + )..createSync(); + + installation.uninstall(executableName); + + expect(rcFile.existsSync(), isTrue); + expect(completionEntry.existsIn(rcFile), isTrue); + + expect(shellCompletionConfigurationFile.existsSync(), isTrue); + expect( + executableEntry.existsIn(shellCompletionConfigurationFile), + isFalse, + ); + expect( + anotherExecutableEntry.existsIn(shellCompletionConfigurationFile), + isTrue, + ); + + expect(executableCompletionConfigurationFile.existsSync(), false); + expect( + anotherExecutableCompletionConfigurationFile.existsSync(), + isTrue, + ); + }); + + group('throws a CompletionUnistallationException', () { + test('when RC file does not exist', () { + final installation = CompletionInstallation( + logger: logger, + isWindows: false, + environment: { + 'HOME': tempDir.path, + }, + configuration: zshConfiguration, + ); + final rcFile = File(path.join(tempDir.path, '.zshrc')); + + expect( + () => installation.uninstall('very_good'), + throwsA( + isA().having( + (e) => e.message, + 'message', + equals('No shell RC file found at ${rcFile.path}'), + ), + ), + ); + }); + + test('when RC file does not have a completion entry', () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final installation = CompletionInstallation( + logger: logger, + isWindows: false, + environment: { + 'HOME': tempDirectory.path, + }, + configuration: zshConfiguration, + ); + + final rcFile = File(path.join(tempDirectory.path, '.zshrc')) + ..createSync(); + + expect( + () => installation.uninstall('very_good'), + throwsA( + isA().having( + (e) => e.message, + 'message', + equals('Completion is not installed at ${rcFile.path}'), + ), + ), + ); + }); + + test('when RC file has a completion entry but no script file', () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final configuration = zshConfiguration; + final installation = CompletionInstallation( + logger: logger, + isWindows: false, + environment: { + 'HOME': tempDirectory.path, + }, + configuration: configuration, + ); + + final rcFile = File(path.join(tempDirectory.path, '.zshrc')) + ..createSync(); + const ScriptConfigurationEntry('Completion').appendTo(rcFile); + + final shellCompletionConfigurationFile = File( + path.join( + installation.completionConfigDir.path, + configuration.completionConfigForShellFileName, + ), + ); + + expect( + () => installation.uninstall('very_good'), + throwsA( + isA().having( + (e) => e.message, + 'message', + equals( + '''No shell script file found at ${shellCompletionConfigurationFile.path}''', + ), + ), + ), + ); + }); + }); + }); }); } diff --git a/test/src/installer/script_configuration_entry_test.dart b/test/src/installer/script_configuration_entry_test.dart index c6e6d62..6da4146 100644 --- a/test/src/installer/script_configuration_entry_test.dart +++ b/test/src/installer/script_configuration_entry_test.dart @@ -16,7 +16,7 @@ void main() { expect(ScriptConfigurationEntry('name').name, 'name'); }); - group('appendsTo', () { + group('appendTo', () { test('returns normally when file exist', () { final tempDirectory = Directory.systemTemp.createTempSync(); addTearDown(() => tempDirectory.deleteSync(recursive: true)); @@ -199,6 +199,24 @@ $initialContent expect(file.existsSync(), isFalse); }); + test( + '''preseves file when there is a single matching entry and should not delete''', + () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final filePath = path.join(tempDirectory.path, 'file'); + final file = File(filePath)..createSync(); + + final entry = ScriptConfigurationEntry('name')..appendTo(file); + expect(entry.existsIn(file), isTrue); + + ScriptConfigurationEntry('name').removeFrom(file, shouldDelete: false); + expect(file.existsSync(), isTrue); + final currentContent = file.readAsStringSync(); + expect(currentContent, isEmpty); + }); + test('''removes file when there is a multiple matching entries''', () { final tempDirectory = Directory.systemTemp.createTempSync(); addTearDown(() => tempDirectory.deleteSync(recursive: true)); From c327c2c2a458b7a53ec0a2d0c9d715b51154d76f Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Sat, 13 May 2023 13:59:05 +0100 Subject: [PATCH 18/29] test: included missing executable script deletion --- .../completion_installation_test.dart | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/test/src/installer/completion_installation_test.dart b/test/src/installer/completion_installation_test.dart index fa5289b..2574a38 100644 --- a/test/src/installer/completion_installation_test.dart +++ b/test/src/installer/completion_installation_test.dart @@ -540,6 +540,43 @@ void main() { expect(executableCompletionConfigurationFile.existsSync(), false); }); + test( + '''deletes entire completion configuration when there is a single executable with a missing completion script''', + () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final configuration = zshConfiguration; + final installation = CompletionInstallation( + logger: logger, + isWindows: false, + environment: { + 'HOME': tempDirectory.path, + }, + configuration: configuration, + ); + + final rcFile = File(path.join(tempDirectory.path, '.zshrc')) + ..createSync(); + const ScriptConfigurationEntry('Completion').appendTo(rcFile); + + final shellCompletionConfigurationFile = File( + path.join( + installation.completionConfigDir.path, + configuration.completionConfigForShellFileName, + ), + ); + const executableName = 'very_good'; + const ScriptConfigurationEntry(executableName) + .appendTo(shellCompletionConfigurationFile); + + installation.uninstall(executableName); + + expect(rcFile.existsSync(), isTrue); + expect(rcFile.readAsStringSync(), isEmpty); + expect(shellCompletionConfigurationFile.existsSync(), false); + }); + test( '''deletes executable completion configuration when there are multiple installed executables''', () { From bfccc39316a328927b598768095bd171bfab768d Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Sat, 13 May 2023 14:23:42 +0100 Subject: [PATCH 19/29] feat: refined uninstallation logic --- .../installer/completion_installation.dart | 12 +- .../completion_installation_test.dart | 120 +++++------------- 2 files changed, 37 insertions(+), 95 deletions(-) diff --git a/lib/src/installer/completion_installation.dart b/lib/src/installer/completion_installation.dart index d2ac205..1865508 100644 --- a/lib/src/installer/completion_installation.dart +++ b/lib/src/installer/completion_installation.dart @@ -358,13 +358,6 @@ ${configuration!.sourceLineTemplate(scriptPath)}'''; ); } - executableEntry.removeFrom(shellCompletionConfigurationFile); - if (!shellCompletionConfigurationFile.existsSync()) { - // TODO(alestiago): Should we check if there are other shells installed, before - // deleting the completion config dir? - completionEntry.removeFrom(shellRCFile, shouldDelete: false); - } - final executableShellCompletionScriptFile = File( path.join( completionConfigDir.path, @@ -375,7 +368,10 @@ ${configuration!.sourceLineTemplate(scriptPath)}'''; executableShellCompletionScriptFile.deleteSync(); } - // TODO(alestiago): Add .uninstall file to avoid autocompletion installing again. + executableEntry.removeFrom(shellCompletionConfigurationFile); + if (!shellCompletionConfigurationFile.existsSync()) { + completionEntry.removeFrom(shellRCFile, shouldDelete: false); + } } } diff --git a/test/src/installer/completion_installation_test.dart b/test/src/installer/completion_installation_test.dart index 2574a38..06a76b6 100644 --- a/test/src/installer/completion_installation_test.dart +++ b/test/src/installer/completion_installation_test.dart @@ -502,6 +502,10 @@ void main() { addTearDown(() => tempDirectory.deleteSync(recursive: true)); final configuration = zshConfiguration; + final rcFile = File(path.join(tempDirectory.path, '.zshrc')) + ..createSync(); + + const executableName = 'very_good'; final installation = CompletionInstallation( logger: logger, isWindows: false, @@ -509,44 +513,27 @@ void main() { 'HOME': tempDirectory.path, }, configuration: configuration, - ); - - final rcFile = File(path.join(tempDirectory.path, '.zshrc')) - ..createSync(); - const ScriptConfigurationEntry('Completion').appendTo(rcFile); - - final shellCompletionConfigurationFile = File( - path.join( - installation.completionConfigDir.path, - configuration.completionConfigForShellFileName, - ), - ); - const executableName = 'very_good'; - const ScriptConfigurationEntry(executableName) - .appendTo(shellCompletionConfigurationFile); - - final executableCompletionConfigurationFile = File( - path.join( - installation.completionConfigDir.path, - '$executableName.zsh', - ), - )..createSync(); - - installation.uninstall(executableName); + ) + ..install(executableName) + ..uninstall(executableName); expect(rcFile.existsSync(), isTrue); expect(rcFile.readAsStringSync(), isEmpty); - expect(shellCompletionConfigurationFile.existsSync(), false); - expect(executableCompletionConfigurationFile.existsSync(), false); + expect(installation.completionConfigDir.existsSync(), false); }); test( - '''deletes entire completion configuration when there is a single executable with a missing completion script''', + '''only deletes executable completion configuration when there are multiple installed executables''', () { final tempDirectory = Directory.systemTemp.createTempSync(); addTearDown(() => tempDirectory.deleteSync(recursive: true)); final configuration = zshConfiguration; + const executableName = 'very_good'; + const anotherExecutableName = 'not_good'; + + final rcFile = File(path.join(tempDirectory.path, '.zshrc')) + ..createSync(); final installation = CompletionInstallation( logger: logger, isWindows: false, @@ -554,11 +541,9 @@ void main() { 'HOME': tempDirectory.path, }, configuration: configuration, - ); - - final rcFile = File(path.join(tempDirectory.path, '.zshrc')) - ..createSync(); - const ScriptConfigurationEntry('Completion').appendTo(rcFile); + ) + ..install(executableName) + ..install(anotherExecutableName); final shellCompletionConfigurationFile = File( path.join( @@ -566,81 +551,42 @@ void main() { configuration.completionConfigForShellFileName, ), ); - const executableName = 'very_good'; - const ScriptConfigurationEntry(executableName) - .appendTo(shellCompletionConfigurationFile); installation.uninstall(executableName); expect(rcFile.existsSync(), isTrue); - expect(rcFile.readAsStringSync(), isEmpty); - expect(shellCompletionConfigurationFile.existsSync(), false); - }); - - test( - '''deletes executable completion configuration when there are multiple installed executables''', - () { - final tempDirectory = Directory.systemTemp.createTempSync(); - addTearDown(() => tempDirectory.deleteSync(recursive: true)); - - final configuration = zshConfiguration; - final installation = CompletionInstallation( - logger: logger, - isWindows: false, - environment: { - 'HOME': tempDirectory.path, - }, - configuration: configuration, + expect( + const ScriptConfigurationEntry('Completion').existsIn(rcFile), + isTrue, ); - final rcFile = File(path.join(tempDirectory.path, '.zshrc')) - ..createSync(); - final completionEntry = const ScriptConfigurationEntry('Completion') - ..appendTo(rcFile); + expect(shellCompletionConfigurationFile.existsSync(), isTrue); + expect( + const ScriptConfigurationEntry(executableName) + .existsIn(shellCompletionConfigurationFile), + isFalse, + ); - final shellCompletionConfigurationFile = File( - path.join( - installation.completionConfigDir.path, - configuration.completionConfigForShellFileName, - ), + expect( + const ScriptConfigurationEntry(anotherExecutableName) + .existsIn(shellCompletionConfigurationFile), + isTrue, ); - const executableName = 'very_good'; - final executableEntry = const ScriptConfigurationEntry(executableName) - ..appendTo(shellCompletionConfigurationFile); + final executableCompletionConfigurationFile = File( path.join( installation.completionConfigDir.path, '$executableName.zsh', ), - )..createSync(); + ); + expect(executableCompletionConfigurationFile.existsSync(), false); - const anotherExecutableName = 'not_good'; - final anotherExecutableEntry = - const ScriptConfigurationEntry(anotherExecutableName) - ..appendTo(shellCompletionConfigurationFile); final anotherExecutableCompletionConfigurationFile = File( path.join( installation.completionConfigDir.path, '$anotherExecutableName.zsh', ), - )..createSync(); - - installation.uninstall(executableName); - - expect(rcFile.existsSync(), isTrue); - expect(completionEntry.existsIn(rcFile), isTrue); - - expect(shellCompletionConfigurationFile.existsSync(), isTrue); - expect( - executableEntry.existsIn(shellCompletionConfigurationFile), - isFalse, ); - expect( - anotherExecutableEntry.existsIn(shellCompletionConfigurationFile), - isTrue, - ); - - expect(executableCompletionConfigurationFile.existsSync(), false); expect( anotherExecutableCompletionConfigurationFile.existsSync(), isTrue, From 11b990e4401f1a06fc06622aac29b35092bfe810 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Sat, 13 May 2023 14:42:01 +0100 Subject: [PATCH 20/29] feat: improved uninstall behaviour for multiple shells --- .../installer/completion_installation.dart | 4 + .../completion_installation_test.dart | 167 ++++++++++++++++-- 2 files changed, 159 insertions(+), 12 deletions(-) diff --git a/lib/src/installer/completion_installation.dart b/lib/src/installer/completion_installation.dart index 1865508..c418670 100644 --- a/lib/src/installer/completion_installation.dart +++ b/lib/src/installer/completion_installation.dart @@ -372,6 +372,10 @@ ${configuration!.sourceLineTemplate(scriptPath)}'''; if (!shellCompletionConfigurationFile.existsSync()) { completionEntry.removeFrom(shellRCFile, shouldDelete: false); } + + if (completionConfigDir.listSync().isEmpty) { + completionConfigDir.deleteSync(); + } } } diff --git a/test/src/installer/completion_installation_test.dart b/test/src/installer/completion_installation_test.dart index 06a76b6..99b4fa1 100644 --- a/test/src/installer/completion_installation_test.dart +++ b/test/src/installer/completion_installation_test.dart @@ -517,9 +517,133 @@ void main() { ..install(executableName) ..uninstall(executableName); - expect(rcFile.existsSync(), isTrue); - expect(rcFile.readAsStringSync(), isEmpty); - expect(installation.completionConfigDir.existsSync(), false); + expect( + rcFile.existsSync(), + isTrue, + reason: 'RC file should not be deleted.', + ); + expect( + const ScriptConfigurationEntry('Completion').existsIn(rcFile), + isFalse, + reason: 'Completion config entry should be removed from RC file.', + ); + expect(installation.completionConfigDir.existsSync(), isFalse); + }); + + test( + '''only deletes shell configuration when there is a single executable in multiple shells''', + () { + final tempDirectory = Directory.systemTemp.createTempSync(); + addTearDown(() => tempDirectory.deleteSync(recursive: true)); + + final zshConfig = zshConfiguration; + final zshRCFile = File(path.join(tempDirectory.path, '.zshrc')) + ..createSync(); + + final bashConfig = bashConfiguration; + final bashRCFile = File(path.join(tempDirectory.path, '.bash_profile')) + ..createSync(); + + const executableName = 'very_good'; + + final bashInstallation = CompletionInstallation( + logger: logger, + isWindows: false, + environment: { + 'HOME': tempDirectory.path, + }, + configuration: bashConfig, + )..install(executableName); + + final zshInstallation = CompletionInstallation( + logger: logger, + isWindows: false, + environment: { + 'HOME': tempDirectory.path, + }, + configuration: zshConfig, + ) + ..install(executableName) + ..uninstall(executableName); + + // Zsh should be uninstalled + expect( + zshRCFile.existsSync(), + isTrue, + reason: 'Zsh RC file should still exist.', + ); + expect( + const ScriptConfigurationEntry('Completion').existsIn(zshRCFile), + isFalse, + reason: 'Zsh should not have completion entry.', + ); + + final zshCompletionConfigurationFile = File( + path.join( + zshInstallation.completionConfigDir.path, + zshConfig.completionConfigForShellFileName, + ), + ); + expect( + zshCompletionConfigurationFile.existsSync(), + isFalse, + reason: 'Zsh completion configuration should be deleted.', + ); + + final zshExecutableCompletionConfigurationFile = File( + path.join( + zshInstallation.completionConfigDir.path, + '$executableName.zsh', + ), + ); + expect( + zshExecutableCompletionConfigurationFile.existsSync(), + isFalse, + reason: 'Zsh executable completion configuration should be deleted.', + ); + + // Bash should still be installed + expect( + bashRCFile.existsSync(), + isTrue, + reason: 'Bash RC file should still exist.', + ); + expect( + const ScriptConfigurationEntry('Completion').existsIn(bashRCFile), + isTrue, + reason: 'Bash should have completion entry.', + ); + + final bashCompletionConfigurationFile = File( + path.join( + bashInstallation.completionConfigDir.path, + bashConfig.completionConfigForShellFileName, + ), + ); + expect( + bashCompletionConfigurationFile.existsSync(), + isTrue, + reason: 'Bash completion configuration should still exist.', + ); + + final bashExecutableCompletionConfigurationFile = File( + path.join( + bashInstallation.completionConfigDir.path, + '$executableName.bash', + ), + ); + expect( + bashExecutableCompletionConfigurationFile.existsSync(), + isTrue, + reason: + 'Bash executable completion configuration should still exist.', + ); + + expect( + bashInstallation.completionConfigDir.existsSync(), + isTrue, + reason: 'Completion configuration directory should still exist.', + ); }); test( @@ -554,33 +678,50 @@ void main() { installation.uninstall(executableName); - expect(rcFile.existsSync(), isTrue); + expect( + rcFile.existsSync(), + isTrue, + reason: 'RC file should not be deleted.', + ); expect( const ScriptConfigurationEntry('Completion').existsIn(rcFile), isTrue, + reason: 'Completion config entry should not be removed from RC file.', ); - expect(shellCompletionConfigurationFile.existsSync(), isTrue); expect( - const ScriptConfigurationEntry(executableName) - .existsIn(shellCompletionConfigurationFile), - isFalse, + shellCompletionConfigurationFile.existsSync(), + isTrue, + reason: 'Shell completion configuration should still exist.', ); expect( - const ScriptConfigurationEntry(anotherExecutableName) + const ScriptConfigurationEntry(executableName) .existsIn(shellCompletionConfigurationFile), - isTrue, + isFalse, + reason: + '''Executable completion for $executableName configuration should be removed.''', ); - final executableCompletionConfigurationFile = File( path.join( installation.completionConfigDir.path, '$executableName.zsh', ), ); - expect(executableCompletionConfigurationFile.existsSync(), false); + expect( + executableCompletionConfigurationFile.existsSync(), + false, + reason: + '''Executable completion configuration for $executableName should be deleted.''', + ); + expect( + const ScriptConfigurationEntry(anotherExecutableName) + .existsIn(shellCompletionConfigurationFile), + isTrue, + reason: + '''Executable completion configuration for $anotherExecutableName should still exist.''', + ); final anotherExecutableCompletionConfigurationFile = File( path.join( installation.completionConfigDir.path, @@ -590,6 +731,8 @@ void main() { expect( anotherExecutableCompletionConfigurationFile.existsSync(), isTrue, + reason: + '''Executable completion configuration for $anotherExecutableName should still exist.''', ); }); From b2db32a73b9c0cec3a66269b7cc16bbcc06dc82b Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Sat, 13 May 2023 14:49:41 +0100 Subject: [PATCH 21/29] test: added CompletionUnistallationException --- ...mpletion_unistallation_exception_test.dart | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 test/src/installer/completion_unistallation_exception_test.dart diff --git a/test/src/installer/completion_unistallation_exception_test.dart b/test/src/installer/completion_unistallation_exception_test.dart new file mode 100644 index 0000000..221364e --- /dev/null +++ b/test/src/installer/completion_unistallation_exception_test.dart @@ -0,0 +1,60 @@ +import 'package:cli_completion/installer.dart'; +import 'package:test/test.dart'; + +void main() { + group('$CompletionUnistallationException', () { + test('can be instantiated', () { + expect( + () => CompletionUnistallationException( + message: 'message', + executableName: 'executableName', + ), + returnsNormally, + ); + }); + + test('has a message', () { + expect( + CompletionUnistallationException( + message: 'message', + executableName: 'executableName', + ).message, + equals('message'), + ); + }); + + test('has an executableName', () { + expect( + CompletionUnistallationException( + message: 'message', + executableName: 'executableName', + ).executableName, + equals('executableName'), + ); + }); + + group('toString', () { + test('returns a string', () { + expect( + CompletionUnistallationException( + message: 'message', + executableName: 'executableName', + ).toString(), + isA(), + ); + }); + + test('returns a correctly formatted string', () { + expect( + CompletionUnistallationException( + message: 'message', + executableName: 'executableName', + ).toString(), + equals( + '''Could not uninstall completion scripts for executableName: message''', + ), + ); + }); + }); + }); +} From ac3a06649ae0feb40a27ea2e1ef051ecc8ebed9a Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Sat, 13 May 2023 14:53:29 +0100 Subject: [PATCH 22/29] docs: improved uninstall documentation --- lib/src/installer/completion_installation.dart | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/lib/src/installer/completion_installation.dart b/lib/src/installer/completion_installation.dart index c418670..b73e68a 100644 --- a/lib/src/installer/completion_installation.dart +++ b/lib/src/installer/completion_installation.dart @@ -311,16 +311,19 @@ ${configuration!.sourceLineTemplate(scriptPath)}'''; /// Before uninstalling, it checks if the completion is installed: /// - The shell has an existing RCFile with a completion /// [ScriptConfigurationEntry]. - /// - The shell has an exisiting configuration config file with a + /// - The shell has an exisiting completion configuration file with a /// [ScriptConfigurationEntry] for the [executableName]. /// /// If any of the above is not true, it throws a /// [CompletionUnistallationException]. /// /// Upon a successful uninstallation the executable [ScriptConfigurationEntry] - /// is removed from the shell config file. If after this removal the latter is - /// empty, it is deleted together with the the executable completion script - /// and the completion [ScriptConfigurationEntry] from the shell RC file. + /// is removed from the shell configuration file. If after this removal the + /// latter is empty, it is deleted together with the the executable completion + /// script and the completion [ScriptConfigurationEntry] from the shell RC + /// file. In the case that there are no other completion scripts installed on + /// other shells the completion config directory is deleted, leaving the + /// user's system as it was before the installation. void uninstall(String executableName) { final configuration = this.configuration!; logger.detail( From 55ee7ec31f7fd7d3dc351590f6e47a6d26c40815 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Sat, 13 May 2023 15:00:22 +0100 Subject: [PATCH 23/29] test: improved names --- test/src/installer/script_configuration_entry_test.dart | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/src/installer/script_configuration_entry_test.dart b/test/src/installer/script_configuration_entry_test.dart index 6da4146..1a9fe33 100644 --- a/test/src/installer/script_configuration_entry_test.dart +++ b/test/src/installer/script_configuration_entry_test.dart @@ -185,7 +185,7 @@ $initialContent expect(content, equals(currentContent)); }); - test('removes file when there is a single matching entry', () { + test('removes file when there is only a single matching entry', () { final tempDirectory = Directory.systemTemp.createTempSync(); addTearDown(() => tempDirectory.deleteSync(recursive: true)); @@ -217,7 +217,8 @@ $initialContent expect(currentContent, isEmpty); }); - test('''removes file when there is a multiple matching entries''', () { + test('''removes file when there are only multiple matching entries''', + () { final tempDirectory = Directory.systemTemp.createTempSync(); addTearDown(() => tempDirectory.deleteSync(recursive: true)); From e249a974d324aa9f461703a9293cd92943eedd1d Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Tue, 16 May 2023 14:46:43 +0100 Subject: [PATCH 24/29] refactor: renamed executablName to rootCommand --- .../installer/completion_installation.dart | 18 ++--- .../completion_installation_test.dart | 71 +++++++++---------- 2 files changed, 44 insertions(+), 45 deletions(-) diff --git a/lib/src/installer/completion_installation.dart b/lib/src/installer/completion_installation.dart index b73e68a..3175241 100644 --- a/lib/src/installer/completion_installation.dart +++ b/lib/src/installer/completion_installation.dart @@ -305,14 +305,14 @@ ${configuration!.sourceLineTemplate(scriptPath)}'''; logger.info('Added config to $configFilePath'); } - /// Uninstalls the completion for the command [executableName] on the current + /// Uninstalls the completion for the command [rootCommand] on the current /// shell. /// /// Before uninstalling, it checks if the completion is installed: /// - The shell has an existing RCFile with a completion /// [ScriptConfigurationEntry]. /// - The shell has an exisiting completion configuration file with a - /// [ScriptConfigurationEntry] for the [executableName]. + /// [ScriptConfigurationEntry] for the [rootCommand]. /// /// If any of the above is not true, it throws a /// [CompletionUnistallationException]. @@ -324,16 +324,16 @@ ${configuration!.sourceLineTemplate(scriptPath)}'''; /// file. In the case that there are no other completion scripts installed on /// other shells the completion config directory is deleted, leaving the /// user's system as it was before the installation. - void uninstall(String executableName) { + void uninstall(String rootCommand) { final configuration = this.configuration!; logger.detail( - '''Uninstalling completion for the command $executableName on ${configuration.shell.name}''', + '''Uninstalling completion for the command $rootCommand on ${configuration.shell.name}''', ); final shellRCFile = File(_shellRCFilePath); if (!shellRCFile.existsSync()) { throw CompletionUnistallationException( - executableName: executableName, + executableName: rootCommand, message: 'No shell RC file found at ${shellRCFile.path}', ); } @@ -341,7 +341,7 @@ ${configuration!.sourceLineTemplate(scriptPath)}'''; const completionEntry = ScriptConfigurationEntry('Completion'); if (!completionEntry.existsIn(shellRCFile)) { throw CompletionUnistallationException( - executableName: executableName, + executableName: rootCommand, message: 'Completion is not installed at ${shellRCFile.path}', ); } @@ -352,10 +352,10 @@ ${configuration!.sourceLineTemplate(scriptPath)}'''; configuration.completionConfigForShellFileName, ), ); - final executableEntry = ScriptConfigurationEntry(executableName); + final executableEntry = ScriptConfigurationEntry(rootCommand); if (!executableEntry.existsIn(shellCompletionConfigurationFile)) { throw CompletionUnistallationException( - executableName: executableName, + executableName: rootCommand, message: '''No shell script file found at ${shellCompletionConfigurationFile.path}''', ); @@ -364,7 +364,7 @@ ${configuration!.sourceLineTemplate(scriptPath)}'''; final executableShellCompletionScriptFile = File( path.join( completionConfigDir.path, - '$executableName.${configuration.shell.name}', + '$rootCommand.${configuration.shell.name}', ), ); if (executableShellCompletionScriptFile.existsSync()) { diff --git a/test/src/installer/completion_installation_test.dart b/test/src/installer/completion_installation_test.dart index 99b4fa1..a7ead70 100644 --- a/test/src/installer/completion_installation_test.dart +++ b/test/src/installer/completion_installation_test.dart @@ -496,7 +496,7 @@ void main() { group('uninstall', () { test( - '''deletes entire completion configuration when there is a single executable''', + '''deletes entire completion configuration when there is a single command''', () { final tempDirectory = Directory.systemTemp.createTempSync(); addTearDown(() => tempDirectory.deleteSync(recursive: true)); @@ -505,7 +505,7 @@ void main() { final rcFile = File(path.join(tempDirectory.path, '.zshrc')) ..createSync(); - const executableName = 'very_good'; + const rootCommand = 'very_good'; final installation = CompletionInstallation( logger: logger, isWindows: false, @@ -514,8 +514,8 @@ void main() { }, configuration: configuration, ) - ..install(executableName) - ..uninstall(executableName); + ..install(rootCommand) + ..uninstall(rootCommand); expect( rcFile.existsSync(), @@ -531,7 +531,7 @@ void main() { }); test( - '''only deletes shell configuration when there is a single executable in multiple shells''', + '''only deletes shell configuration when there is a single command in multiple shells''', () { final tempDirectory = Directory.systemTemp.createTempSync(); addTearDown(() => tempDirectory.deleteSync(recursive: true)); @@ -544,7 +544,7 @@ void main() { final bashRCFile = File(path.join(tempDirectory.path, '.bash_profile')) ..createSync(); - const executableName = 'very_good'; + const rootCommand = 'very_good'; final bashInstallation = CompletionInstallation( logger: logger, @@ -553,7 +553,7 @@ void main() { 'HOME': tempDirectory.path, }, configuration: bashConfig, - )..install(executableName); + )..install(rootCommand); final zshInstallation = CompletionInstallation( logger: logger, @@ -563,8 +563,8 @@ void main() { }, configuration: zshConfig, ) - ..install(executableName) - ..uninstall(executableName); + ..install(rootCommand) + ..uninstall(rootCommand); // Zsh should be uninstalled expect( @@ -590,16 +590,16 @@ void main() { reason: 'Zsh completion configuration should be deleted.', ); - final zshExecutableCompletionConfigurationFile = File( + final zshCommandCompletionConfigurationFile = File( path.join( zshInstallation.completionConfigDir.path, - '$executableName.zsh', + '$rootCommand.zsh', ), ); expect( - zshExecutableCompletionConfigurationFile.existsSync(), + zshCommandCompletionConfigurationFile.existsSync(), isFalse, - reason: 'Zsh executable completion configuration should be deleted.', + reason: 'Zsh command completion configuration should be deleted.', ); // Bash should still be installed @@ -626,17 +626,16 @@ void main() { reason: 'Bash completion configuration should still exist.', ); - final bashExecutableCompletionConfigurationFile = File( + final bashCommandCompletionConfigurationFile = File( path.join( bashInstallation.completionConfigDir.path, - '$executableName.bash', + '$rootCommand.bash', ), ); expect( - bashExecutableCompletionConfigurationFile.existsSync(), + bashCommandCompletionConfigurationFile.existsSync(), isTrue, - reason: - 'Bash executable completion configuration should still exist.', + reason: 'Bash command completion configuration should still exist.', ); expect( @@ -647,14 +646,14 @@ void main() { }); test( - '''only deletes executable completion configuration when there are multiple installed executables''', + '''only deletes command completion configuration when there are multiple installed commands''', () { final tempDirectory = Directory.systemTemp.createTempSync(); addTearDown(() => tempDirectory.deleteSync(recursive: true)); final configuration = zshConfiguration; - const executableName = 'very_good'; - const anotherExecutableName = 'not_good'; + const commandName = 'very_good'; + const anotherCommandName = 'not_good'; final rcFile = File(path.join(tempDirectory.path, '.zshrc')) ..createSync(); @@ -666,8 +665,8 @@ void main() { }, configuration: configuration, ) - ..install(executableName) - ..install(anotherExecutableName); + ..install(commandName) + ..install(anotherCommandName); final shellCompletionConfigurationFile = File( path.join( @@ -676,7 +675,7 @@ void main() { ), ); - installation.uninstall(executableName); + installation.uninstall(commandName); expect( rcFile.existsSync(), @@ -696,43 +695,43 @@ void main() { ); expect( - const ScriptConfigurationEntry(executableName) + const ScriptConfigurationEntry(commandName) .existsIn(shellCompletionConfigurationFile), isFalse, reason: - '''Executable completion for $executableName configuration should be removed.''', + '''Command completion for $commandName configuration should be removed.''', ); - final executableCompletionConfigurationFile = File( + final commandCompletionConfigurationFile = File( path.join( installation.completionConfigDir.path, - '$executableName.zsh', + '$commandName.zsh', ), ); expect( - executableCompletionConfigurationFile.existsSync(), + commandCompletionConfigurationFile.existsSync(), false, reason: - '''Executable completion configuration for $executableName should be deleted.''', + '''Command completion configuration for $commandName should be deleted.''', ); expect( - const ScriptConfigurationEntry(anotherExecutableName) + const ScriptConfigurationEntry(anotherCommandName) .existsIn(shellCompletionConfigurationFile), isTrue, reason: - '''Executable completion configuration for $anotherExecutableName should still exist.''', + '''Command completion configuration for $anotherCommandName should still exist.''', ); - final anotherExecutableCompletionConfigurationFile = File( + final anotherCommandCompletionConfigurationFile = File( path.join( installation.completionConfigDir.path, - '$anotherExecutableName.zsh', + '$anotherCommandName.zsh', ), ); expect( - anotherExecutableCompletionConfigurationFile.existsSync(), + anotherCommandCompletionConfigurationFile.existsSync(), isTrue, reason: - '''Executable completion configuration for $anotherExecutableName should still exist.''', + '''Command completion configuration for $anotherCommandName should still exist.''', ); }); From 96b27969d0c72f3e3db53d526cbe21110ddfdb4e Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Tue, 16 May 2023 15:39:07 +0100 Subject: [PATCH 25/29] refactor: made shouldDelete false by default --- lib/src/installer/completion_installation.dart | 7 +++++-- .../installer/script_configuration_entry.dart | 2 +- .../script_configuration_entry_test.dart | 17 ++++++++++------- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/lib/src/installer/completion_installation.dart b/lib/src/installer/completion_installation.dart index 3175241..3d8115a 100644 --- a/lib/src/installer/completion_installation.dart +++ b/lib/src/installer/completion_installation.dart @@ -371,9 +371,12 @@ ${configuration!.sourceLineTemplate(scriptPath)}'''; executableShellCompletionScriptFile.deleteSync(); } - executableEntry.removeFrom(shellCompletionConfigurationFile); + executableEntry.removeFrom( + shellCompletionConfigurationFile, + shouldDelete: true, + ); if (!shellCompletionConfigurationFile.existsSync()) { - completionEntry.removeFrom(shellRCFile, shouldDelete: false); + completionEntry.removeFrom(shellRCFile); } if (completionConfigDir.listSync().isEmpty) { diff --git a/lib/src/installer/script_configuration_entry.dart b/lib/src/installer/script_configuration_entry.dart index 1f94447..3eb38f7 100644 --- a/lib/src/installer/script_configuration_entry.dart +++ b/lib/src/installer/script_configuration_entry.dart @@ -61,7 +61,7 @@ class ScriptConfigurationEntry { /// /// If [shouldDelete] is true, the [file] will be deleted if it is empty after /// removing the entry. Otherwise, the [file] will be left empty. - void removeFrom(File file, {bool shouldDelete = true}) { + void removeFrom(File file, {bool shouldDelete = false}) { if (!file.existsSync()) return; final content = file.readAsStringSync(); diff --git a/test/src/installer/script_configuration_entry_test.dart b/test/src/installer/script_configuration_entry_test.dart index 1a9fe33..6a29b32 100644 --- a/test/src/installer/script_configuration_entry_test.dart +++ b/test/src/installer/script_configuration_entry_test.dart @@ -157,14 +157,14 @@ $initialContent expect(() => entry.removeFrom(file), returnsNormally); }); - test('deletes file when file is empty', () { + test('deletes file when file is empty and should delete', () { final tempDirectory = Directory.systemTemp.createTempSync(); addTearDown(() => tempDirectory.deleteSync(recursive: true)); final filePath = path.join(tempDirectory.path, 'file'); final file = File(filePath)..createSync(); - ScriptConfigurationEntry('name').removeFrom(file); + ScriptConfigurationEntry('name').removeFrom(file, shouldDelete: true); expect(file.existsSync(), isFalse); }); @@ -185,7 +185,9 @@ $initialContent expect(content, equals(currentContent)); }); - test('removes file when there is only a single matching entry', () { + test( + '''removes file when there is only a single matching entry and should delete''', + () { final tempDirectory = Directory.systemTemp.createTempSync(); addTearDown(() => tempDirectory.deleteSync(recursive: true)); @@ -195,7 +197,7 @@ $initialContent final entry = ScriptConfigurationEntry('name')..appendTo(file); expect(entry.existsIn(file), isTrue); - ScriptConfigurationEntry('name').removeFrom(file); + ScriptConfigurationEntry('name').removeFrom(file, shouldDelete: true); expect(file.existsSync(), isFalse); }); @@ -211,13 +213,14 @@ $initialContent final entry = ScriptConfigurationEntry('name')..appendTo(file); expect(entry.existsIn(file), isTrue); - ScriptConfigurationEntry('name').removeFrom(file, shouldDelete: false); + ScriptConfigurationEntry('name').removeFrom(file); expect(file.existsSync(), isTrue); final currentContent = file.readAsStringSync(); expect(currentContent, isEmpty); }); - test('''removes file when there are only multiple matching entries''', + test( + '''removes file when there are only multiple matching entries and should delete''', () { final tempDirectory = Directory.systemTemp.createTempSync(); addTearDown(() => tempDirectory.deleteSync(recursive: true)); @@ -231,7 +234,7 @@ $initialContent ..appendTo(file); expect(entry.existsIn(file), isTrue); - ScriptConfigurationEntry('name').removeFrom(file); + ScriptConfigurationEntry('name').removeFrom(file, shouldDelete: true); expect(file.existsSync(), isFalse); }); From db0b752a44b2727cbe0c0f090de2dfc10b1330d1 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Tue, 16 May 2023 15:39:31 +0100 Subject: [PATCH 26/29] docs: fixed wrong documentation --- lib/src/installer/exceptions.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/installer/exceptions.dart b/lib/src/installer/exceptions.dart index d7bc58a..6c481b6 100644 --- a/lib/src/installer/exceptions.dart +++ b/lib/src/installer/exceptions.dart @@ -20,7 +20,7 @@ class CompletionInstallationException implements Exception { } /// {@template completion_unistallation_exception} -/// Describes an exception during the installation of completion scripts. +/// Describes an exception during the uninstallation of completion scripts. /// {@endtemplate} class CompletionUnistallationException implements Exception { /// {@macro completion_unistallation_exception} From cba71745a0dda28e0cf58d29fa54095e58925adb Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Tue, 16 May 2023 15:39:51 +0100 Subject: [PATCH 27/29] test: renamed file --- ...mpletion_unistallation_exception_test.dart => exceptions.dart} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/src/installer/{completion_unistallation_exception_test.dart => exceptions.dart} (100%) diff --git a/test/src/installer/completion_unistallation_exception_test.dart b/test/src/installer/exceptions.dart similarity index 100% rename from test/src/installer/completion_unistallation_exception_test.dart rename to test/src/installer/exceptions.dart From 0736ca5dc77198fca969203a07c49b1983c06bb3 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Tue, 16 May 2023 15:42:39 +0100 Subject: [PATCH 28/29] test: include _test in test name --- test/src/installer/{exceptions.dart => exceptions_test.dart} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test/src/installer/{exceptions.dart => exceptions_test.dart} (100%) diff --git a/test/src/installer/exceptions.dart b/test/src/installer/exceptions_test.dart similarity index 100% rename from test/src/installer/exceptions.dart rename to test/src/installer/exceptions_test.dart From bc847d46e6f523ad6985af8d1d7eea54c8e02c73 Mon Sep 17 00:00:00 2001 From: Alejandro Santiago Date: Wed, 17 May 2023 15:29:47 +0100 Subject: [PATCH 29/29] Update lib/src/installer/completion_installation.dart Co-authored-by: Renan <6718144+renancaraujo@users.noreply.github.com> --- lib/src/installer/completion_installation.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/src/installer/completion_installation.dart b/lib/src/installer/completion_installation.dart index 3d8115a..7552ddc 100644 --- a/lib/src/installer/completion_installation.dart +++ b/lib/src/installer/completion_installation.dart @@ -311,7 +311,7 @@ ${configuration!.sourceLineTemplate(scriptPath)}'''; /// Before uninstalling, it checks if the completion is installed: /// - The shell has an existing RCFile with a completion /// [ScriptConfigurationEntry]. - /// - The shell has an exisiting completion configuration file with a + /// - The shell has an existing completion configuration file with a /// [ScriptConfigurationEntry] for the [rootCommand]. /// /// If any of the above is not true, it throws a