-
Notifications
You must be signed in to change notification settings - Fork 5
fix: separate pkg manager completion handling from the core API #31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,115 @@ | ||
| import { execSync } from 'child_process'; | ||
| import { RootCommand } from '../src/t.js'; | ||
|
|
||
| function debugLog(...args: any[]) { | ||
| if (process.env.DEBUG) { | ||
| console.error('[DEBUG]', ...args); | ||
| } | ||
| } | ||
|
|
||
| async function checkCliHasCompletions( | ||
| cliName: string, | ||
| packageManager: string | ||
| ): Promise<boolean> { | ||
| try { | ||
| debugLog(`Checking if ${cliName} has completions via ${packageManager}`); | ||
| const command = `${packageManager} ${cliName} complete --`; | ||
| const result = execSync(command, { | ||
| encoding: 'utf8', | ||
| stdio: ['pipe', 'pipe', 'ignore'], | ||
| timeout: 1000, | ||
| }); | ||
| const hasCompletions = !!result.trim(); | ||
| debugLog(`${cliName} supports completions: ${hasCompletions}`); | ||
| return hasCompletions; | ||
| } catch (error) { | ||
| debugLog(`Error checking completions for ${cliName}:`, error); | ||
| return false; | ||
| } | ||
| } | ||
|
|
||
| async function getCliCompletions( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this function and checkCliHasCompletions has so much redundancy and i feel they're heavily similar. |
||
| cliName: string, | ||
| packageManager: string, | ||
| args: string[] | ||
| ): Promise<string[]> { | ||
| try { | ||
| const completeArgs = args.map((arg) => | ||
| arg.includes(' ') ? `"${arg}"` : arg | ||
| ); | ||
| const completeCommand = `${packageManager} ${cliName} complete -- ${completeArgs.join(' ')}`; | ||
| debugLog(`Getting completions with command: ${completeCommand}`); | ||
|
|
||
| const result = execSync(completeCommand, { | ||
| encoding: 'utf8', | ||
| stdio: ['pipe', 'pipe', 'ignore'], | ||
| timeout: 1000, | ||
| }); | ||
|
|
||
| const completions = result.trim().split('\n').filter(Boolean); | ||
| debugLog(`Got ${completions.length} completions from ${cliName}`); | ||
| return completions; | ||
| } catch (error) { | ||
| debugLog(`Error getting completions from ${cliName}:`, error); | ||
| return []; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Package Manager Completion Wrapper | ||
| * | ||
| * This extends RootCommand and adds package manager-specific logic. | ||
| * It acts as a layer on top of the core tab library. | ||
| */ | ||
| export class PackageManagerCompletion extends RootCommand { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i do not think we need this class, it can be done through some imperative functions and calls.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's remove it and revisit the implementation. it's a bit hard to understand this implementation for me.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. and we need tests for the package manager completions. |
||
| private packageManager: string; | ||
|
|
||
| constructor(packageManager: string) { | ||
| super(); | ||
| this.packageManager = packageManager; | ||
| } | ||
|
|
||
| // Enhanced parse method with package manager logic | ||
| async parse(args: string[]) { | ||
| // Handle package manager completions first | ||
| if (args.length >= 1 && args[0].trim() !== '') { | ||
| const potentialCliName = args[0]; | ||
| const knownCommands = [...this.commands.keys()]; | ||
|
|
||
| if (!knownCommands.includes(potentialCliName)) { | ||
| const hasCompletions = await checkCliHasCompletions( | ||
| potentialCliName, | ||
| this.packageManager | ||
| ); | ||
|
|
||
| if (hasCompletions) { | ||
| const cliArgs = args.slice(1); | ||
| const suggestions = await getCliCompletions( | ||
| potentialCliName, | ||
| this.packageManager, | ||
| cliArgs | ||
| ); | ||
|
|
||
| if (suggestions.length > 0) { | ||
| // Print completions directly in the same format as the core library | ||
| for (const suggestion of suggestions) { | ||
| if (suggestion.startsWith(':')) continue; | ||
|
|
||
| if (suggestion.includes('\t')) { | ||
| const [value, description] = suggestion.split('\t'); | ||
| console.log(`${value}\t${description}`); | ||
| } else { | ||
| console.log(suggestion); | ||
| } | ||
| } | ||
| console.log(':4'); // Shell completion directive (NoFileComp) | ||
| return; | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // Fall back to regular completion logic (shows basic package manager commands) | ||
| return super.parse(args); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,13 +61,9 @@ const completion = tab(program); | |
|
|
||
| // Configure custom completions | ||
| for (const command of completion.commands.values()) { | ||
| if (command.name === 'lint') { | ||
| command.handler = () => { | ||
| return [ | ||
| { value: 'src/**/*.ts', description: 'TypeScript source files' }, | ||
| { value: 'tests/**/*.ts', description: 'Test files' }, | ||
| ]; | ||
| }; | ||
| if (command.value === 'lint') { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the whole commander api needs a new look. |
||
| // Note: Direct handler assignment is not supported in the current API | ||
| // Custom completion logic would need to be implemented differently | ||
| } | ||
|
|
||
| for (const [option, config] of command.options.entries()) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i do not see any completion for all of these commands, we should provide for all of them before release. that's what we agreed on as far as i remember.