Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,11 @@

## 0.4.0-dev

- feat: Added a dialog to select which packages to install skills from during
`skills get` and `skills remove`.
- feat: Support passing multiple package names as trailing arguments.
- **Breaking Change**: `getSkills` now takes a set of package names to install
instead of just a single package name.
- refactor: Migrate from `.agent/skills` to `.agents/skills` for the generic IDE
adapter. When a `.agent/` dir is detected you will be prompted for what action
to take.
Expand Down
2 changes: 1 addition & 1 deletion lib/src/commands/get_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class GetCommand extends SkillsCommand {
dialogSupport: _dialogSupport,
gitRunner: _effectiveGitRunner,
usage: usage,
packageName: packageNameArg,
packageNames: packageNamesArg?.toSet(),
);
}
}
64 changes: 57 additions & 7 deletions lib/src/commands/get_skills.dart
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,16 @@ import 'package:skills/src/models/global_config.dart';
import '../models/skill_manifest.dart';

/// Installs skills from package dependencies for [ides].
///
/// Returns `true` on success or `false` otherwise.
Future<bool> getSkills({
required List<Ide> ides,
required Logger logger,
required WorkspaceLayout workspace,
DialogSupport? dialogSupport,
GitRunner gitRunner = const GitRunner(),
String usage = '',
String? packageName,
Set<String>? packageNames,
}) async {
final ready = await PubRunner.ensureWorkspaceConfigs(workspace);
if (!ready) {
Expand All @@ -37,12 +39,23 @@ Future<bool> getSkills({

final packages = await PackageResolver.resolveWorkspace(
workspace,
packageName: packageName,
packageNames: packageNames,
);

if (packageName != null && packages.isEmpty) {
logger.severe('Package "$packageName" not found in dependencies.');
return false;
if (packageNames != null) {
if (packages.isEmpty) {
logger
.severe('None of the requested packages were found in dependencies.');
return false;
}

final foundNames = packages.map((p) => p.name).toSet();
final missing = packageNames.difference(foundNames)..remove('all');
if (missing.isNotEmpty) {
logger.warning(
'Warning: The following requested packages were not found in '
'dependencies: ${missing.join(', ')}');
}
}

final rootPath = workspace.rootPath;
Expand Down Expand Up @@ -109,14 +122,51 @@ Future<bool> getSkills({
final dartSkills = await scanner.scan(packages);

final resolvedPackageNames = packages.map((p) => p.name).toSet();
final skills = mergeSkills(
var skills = mergeSkills(
dartSkills: dartSkills,
registrySkills: registrySkills,
resolvedPackageNames: resolvedPackageNames,
);

if (skills.isEmpty) {
logger.info('No skills found in ${packageName ?? "any"} packages.');
logger.info('No skills found in ${packageNames ?? "any"} packages.');
return false;
}

if (packageNames == null) {
final packagesWithSkills =
skills.map((skill) => skill.packageName).toSet().toList()..sort();
if (packagesWithSkills.isNotEmpty) {
if (dialogSupport != null) {
final initialSelected =
Iterable<int>.generate(packagesWithSkills.length).toSet();
final selectedIndices = await dialogSupport.showMultiSelectDialog(
packagesWithSkills,
title: 'Select packages to install skills from:',
initialSelected: initialSelected,
);
if (selectedIndices != null) {
final selectedPackages =
selectedIndices.map((i) => packagesWithSkills[i]).toSet();
skills.removeWhere((s) => !selectedPackages.contains(s.packageName));
} else {
logger.info('Installation aborted by user.');
return false;
}
Comment thread
jakemac53 marked this conversation as resolved.
} else {
logger.info('Available packages with skills:');
for (final pkg in packagesWithSkills) {
logger.info(' $pkg');
}
logger.info('Rerun with trailing arguments for each package you want '
'to install skills for, or `all` to install all skills.');
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

if there is no dialogSupport, should we just default to all to mimic the previous behavior of skills get?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I don't think so - I want that to be more explicit personally. We do have the all special argument to handle this case.

Primarily this is here for agentic usage and I think this message should help agents in terms of choosing which skills to actually install on the next call. Maybe they will just pass all, or maybe they will be a bit more direct, but it forces a choice.

return false;
}
}
}

if (skills.isEmpty) {
logger.info('No skills selected to install.');
return false;
}

Expand Down
32 changes: 15 additions & 17 deletions lib/src/commands/prune_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -65,28 +65,26 @@ class PruneCommand extends SkillsCommand {

for (final ide in targetIdes) {
final pkgs = manifest.packagesForIde(ide.cliName);
for (final packageName in pkgs.keys) {
if (referencedNames.contains(packageName)) continue;

final result = await installer.removeSkillsForIde(
ide: ide,
rootPath: rootPath,
manifest: manifest,
packageName: packageName,
);
manifest = result.manifest;
totalRemoved += result.removedCount;
prunedPackages.add(packageName);
for (final info in result.removed) {
logger.info(' [${info.ideName}] Removed ${info.skillName}');
}
final pkgsToPrune =
pkgs.keys.where((name) => !referencedNames.contains(name)).toSet();
prunedPackages.addAll(pkgsToPrune);

final result = await installer.removeSkillsForIde(
ide: ide,
rootPath: rootPath,
manifest: manifest,
packageNames: pkgsToPrune,
);
manifest = result.manifest;
totalRemoved += result.removedCount;
for (final info in result.removed) {
logger.info(' [${info.ideName}] Removed ${info.skillName}');
}
}

await manifest.save(manifestFile(rootPath));
if (manifest.isEmpty) {
await SkillManifest.cleanup(rootPath);
} else {
await manifest.save(manifestFile(rootPath));
}

if (totalRemoved == 0) {
Expand Down
52 changes: 46 additions & 6 deletions lib/src/commands/remove_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ class RemoveCommand extends SkillsCommand {

var manifest = loaded;

final packageName = packageNameArg;
var packagesToRemove = packageNamesArg?.toSet();

// Determine which IDEs to remove from: --ide narrows to one,
// otherwise all IDEs in the manifest.
Expand All @@ -50,6 +50,46 @@ class RemoveCommand extends SkillsCommand {
.toList();
}

if (packagesToRemove == null) {
final packagesWithSkills = <String>{};
for (final ide in targetIdes) {
packagesWithSkills.addAll(manifest.packagesForIde(ide.cliName).keys);
}
final packagesList = packagesWithSkills.toList()..sort();

if (packagesList.isEmpty) {
logger.info('No skills found to remove.');
return;
}

if (_dialogSupport != null) {
final selectedIndices = await _dialogSupport.showMultiSelectDialog(
packagesList,
title: 'Select packages to remove skills for:',
);
if (selectedIndices != null) {
packagesToRemove =
selectedIndices.map((i) => packagesList[i]).toSet();
} else {
logger.info('Removal aborted.');
return;
}
} else {
logger.info('Packages with installed skills:');
for (final pkg in packagesList) {
logger.info(' $pkg');
}
logger.info('Rerun with trailing arguments for each package you want '
'to remove skills for, or `all` to remove all skills.');
return;
}
}

if (packagesToRemove.isEmpty) {
logger.info('No packages selected for removal.');
return;
}

final installer = SkillInstaller(_dialogSupport);
var totalRemoved = 0;

Expand All @@ -58,7 +98,7 @@ class RemoveCommand extends SkillsCommand {
ide: ide,
rootPath: rootPath,
manifest: manifest,
packageName: packageName,
packageNames: packagesToRemove,
);
manifest = result.manifest;
totalRemoved += result.removedCount;
Expand All @@ -67,14 +107,14 @@ class RemoveCommand extends SkillsCommand {
}
}

await manifest.save(manifestFile(rootPath));
if (manifest.isEmpty) {
await SkillManifest.cleanup(rootPath);
} else {
await manifest.save(manifestFile(rootPath));
}

if (packageName != null) {
logger.info('Removed skills from $packageName.');
if (totalRemoved > 0) {
logger.info('Removed $totalRemoved skill(s) from '
'${packagesToRemove.join(', ')}.');
} else {
logger.info('Removed $totalRemoved managed skill(s).');
}
Expand Down
10 changes: 5 additions & 5 deletions lib/src/commands/skills_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,11 @@ abstract class SkillsCommand extends Command<void> {
return const WorkspaceResolver().resolve(path);
}

/// The package name from rest arguments, or null if not specified.
String? get packageNameArg =>
argResults != null && argResults!.rest.isNotEmpty
? argResults!.rest.first
: null;
/// The package names from rest arguments, or null if not specified.
List<String>? get packageNamesArg => switch (argResults?.rest) {
List<String> rest when rest.isNotEmpty => rest,
_ => null,
};
}

/// Returns the manifest file for the given [rootPath].
Expand Down
7 changes: 5 additions & 2 deletions lib/src/core/cli_dialog_support.dart
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,11 @@ class CliUtilDialogSupport implements DialogSupport {
}

@override
Future<Set<int>?> showMultiSelectDialog(List<String> options,
{String? title}) async {
Future<Set<int>?> showMultiSelectDialog(
List<String> options, {
String? title,
Set<int> initialSelected = const {},
}) async {
if (title != null) io.stdout.writeln(title);
final result = await cli.showMultiSelectDialog(options, _sharedStdIn);
if (result != null) {
Expand Down
9 changes: 7 additions & 2 deletions lib/src/core/dialog_support.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,11 @@ abstract interface class DialogSupport {
/// cancelled or not implemented.
///
/// The [title] will be shown in an implementation specific way if given.
Future<Set<int>?> showMultiSelectDialog(List<String> options,
{String? title});
///
/// If given, [initialSelected] are the initially selected indices.
Future<Set<int>?> showMultiSelectDialog(
List<String> options, {
String? title,
Set<int> initialSelected = const {},
});
}
18 changes: 10 additions & 8 deletions lib/src/core/package_resolver.dart
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import 'dart:io';

import 'package:package_config/package_config.dart';
import 'package:path/path.dart' as p;

import 'workspace_resolver.dart';

Expand Down Expand Up @@ -58,10 +57,11 @@ class PackageResolver {
/// out workspace member packages (those are the user's own code, not
/// external dependencies that might ship skills).
///
/// If [packageName] is provided, only that package is returned.
/// If [packageNames] is provided, only those packages are returned.
/// If [packageNames] contains 'all', all packages are returned.
static Future<List<ResolvedPackage>> resolveWorkspace(
WorkspaceLayout workspace, {
String? packageName,
Set<String>? packageNames,
}) async {
final memberNames = workspace.packages.map((p) => p.name).toSet();

Expand All @@ -76,14 +76,16 @@ class PackageResolver {
final configFile = File(configPath);
if (!configFile.existsSync()) continue;

final configDir = Directory(p.dirname(p.dirname(configPath)));
final config = await findPackageConfig(configDir);
if (config == null) continue;

final config = await loadPackageConfig(configFile);
for (final package in config.packages) {
if (memberNames.contains(package.name)) continue;
if (seen.contains(package.name)) continue;
if (packageName != null && package.name != packageName) continue;

if (packageNames != null &&
!packageNames.contains('all') &&
!packageNames.contains(package.name)) {
continue;
}

final rootUri = package.root;
if (rootUri.scheme != 'file') continue;
Expand Down
37 changes: 12 additions & 25 deletions lib/src/core/skill_installer.dart
Original file line number Diff line number Diff line change
Expand Up @@ -183,40 +183,27 @@ class SkillInstaller {
}

/// Removes skills for [ide] from [manifest].
/// If [packageName] is set, only that package is removed; otherwise all.
///
/// If [packageNames] is set, only those packages are removed; otherwise all.
/// If [packageNames] contains `all`, then all packages are also removed.
Future<SkillRemoveResult> removeSkillsForIde({
required Ide ide,
required String rootPath,
required SkillManifest manifest,
String? packageName,
Set<String>? packageNames,
}) async {
final adapter = createIdeAdapter(ide, rootPath, _dialogSupport);
final removed = <RemovedSkillInfo>[];

if (packageName != null) {
final pkgEntry = manifest.packagesForIde(ide.cliName)[packageName];
if (pkgEntry == null) {
return SkillRemoveResult(
manifest: manifest,
removedCount: 0,
removed: [],
);
}
for (final skill in pkgEntry.skills) {
await adapter.removeSkill(skill.name);
removed.add(
RemovedSkillInfo(ideName: ide.cliName, skillName: skill.name),
);
}
return SkillRemoveResult(
manifest: manifest.withoutPackage(ide.cliName, packageName),
removedCount: removed.length,
removed: removed,
);
}

final pkgs = manifest.packagesForIde(ide.cliName);

for (final entry in pkgs.entries) {
if (packageNames != null &&
!packageNames.contains('all') &&
!packageNames.contains(entry.key)) {
continue;
}
manifest = manifest.withoutPackage(ide.cliName, entry.key);
for (final skill in entry.value.skills) {
await adapter.removeSkill(skill.name);
removed.add(
Expand All @@ -225,7 +212,7 @@ class SkillInstaller {
}
}
return SkillRemoveResult(
manifest: manifest.withoutIde(ide.cliName),
manifest: manifest,
removedCount: removed.length,
removed: removed,
);
Expand Down
Loading
Loading