Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

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

fix: add zsh autocomplete setup and file permissions instructions to completion:install #6882

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
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
49 changes: 46 additions & 3 deletions src/commands/completion/completion.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import fs from 'fs'
import { homedir } from 'os'
import { dirname, join } from 'path'
import { fileURLToPath } from 'url'
import inquirer from 'inquirer'

import { OptionValues } from 'commander'
// @ts-expect-error TS(7016) FIXME: Could not find a declaration file for module 'tabt... Remove this comment to see the full error message
import { install, uninstall } from 'tabtab'

import { generateAutocompletion } from '../../lib/completion/index.js'
import { error } from '../../utils/command-helpers.js'
import { error, log, chalk, checkFileForLine } from '../../utils/command-helpers.js'
import BaseCommand from '../base-command.js'

const completer = join(dirname(fileURLToPath(import.meta.url)), '../../lib/completion/script.js')
Expand All @@ -20,13 +23,53 @@ export const completionGenerate = async (options: OptionValues, command: BaseCom
}

generateAutocompletion(parent)

await install({
name: parent.name(),
completer,
})

console.log(`Completion for ${parent.name()} successful installed!`)
const TABTAB_CONFIG_LINE = '[[ -f ~/.config/tabtab/__tabtab.zsh ]] && . ~/.config/tabtab/__tabtab.zsh || true'
const AUTOLOAD_COMPINIT = 'autoload -U compinit; compinit'
Comment on lines +31 to +32
Copy link
Contributor

Choose a reason for hiding this comment

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

Throw these in the constants file in this folder

const zshConfigFilepath = join(process.env.HOME || homedir(), '.zshrc')

if (
fs.existsSync(zshConfigFilepath) &&
checkFileForLine(zshConfigFilepath, TABTAB_CONFIG_LINE) &&
!checkFileForLine(zshConfigFilepath, AUTOLOAD_COMPINIT)
) {
log(`To enable Tabtab autocompletion with zsh, the following line may need to be added to your ~/.zshrc:`)
log(chalk.bold.cyan(`\n${AUTOLOAD_COMPINIT}\n`))
await inquirer
.prompt([
{
type: 'confirm',
name: 'compinitAdded',
message: `Would you like to add it?`,
default: true,
},
])
.then((answer) => {
Comment on lines +42 to +51
Copy link
Contributor

Choose a reason for hiding this comment

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

Generally you wouldn't use await and .then() you would use one or the other. If you want it to be blocking use await, if you want it to not be blocking use .then()

Suggested change
await inquirer
.prompt([
{
type: 'confirm',
name: 'compinitAdded',
message: `Would you like to add it?`,
default: true,
},
])
.then((answer) => {
const answer = await inquirer
.prompt([
{
type: 'confirm',
name: 'compinitAdded',
message: `Would you like to add it?`,
default: true,
},
])

if (answer['compinitAdded']) {
fs.readFile(zshConfigFilepath, 'utf8', (err, data) => {
Copy link
Contributor Author

@benhancock benhancock Oct 17, 2024

Choose a reason for hiding this comment

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

Note that we add the autoload -U compinit; compinit line to the top of the ~/.zshrc file (and not the bottom or right before the tabtab config line) because

  1. it needs to be run before the tabtab config line ('[[ -f ~/.config/tabtab/__tabtab.zsh ]] && . ~/.config/tabtab/__tabtab.zsh || true') and
  2. because compinit is also needed for other completions in general (besides netlify/tabtab) to work with zsh

Copy link
Contributor

Choose a reason for hiding this comment

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

I would await this. So await fs.readFile or use fs.readFileSync.

const updatedZshFile = AUTOLOAD_COMPINIT + '\n' + data

fs.writeFileSync(zshConfigFilepath, updatedZshFile, 'utf8')
})

log('Successfully added compinit line to .zshrc')
}
})
}

log(`Completion for ${parent.name()} successfully installed!`)

if (process.platform !== 'win32') {
log("\nTo ensure proper functionality, you'll need to set appropriate file permissions.")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because chmod is a Unix command, we only output this instruction if the user is not on windows

log(chalk.bold('Add executable permissions by running the following command:'))
log(chalk.bold.cyan(`\nchmod +x ${completer}\n`))
} else {
log(`\nTo ensure proper functionality, you may need to set appropriate file permissions to ${completer}.`)
}
}

export const completionUninstall = async (options: OptionValues, command: BaseCommand) => {
Expand Down
12 changes: 12 additions & 0 deletions src/utils/command-helpers.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { once } from 'events'
import os from 'os'
import fs from 'fs'
import process from 'process'
import { format, inspect } from 'util'

Expand Down Expand Up @@ -300,3 +301,14 @@ export const nonNullable = <T>(value: T): value is NonNullable<T> => value !== n
export const noOp = () => {
// no-op
}

export const checkFileForLine = (filename: string, line: string) => {
let filecontent = ''
try {
filecontent = fs.readFileSync(filename, 'utf8')
} catch (error_) {
// @ts-expect-error TS(2571) FIXME: Object is of type 'unknown'.
Copy link
Contributor

Choose a reason for hiding this comment

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

Pull in the main branch to get updates from this PR #6877 to fix this type error

error(error_)
}
return !!filecontent.match(`${line}`)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
import { describe, expect, test, beforeAll, afterAll } from 'vitest'
import fs from 'fs'
import { rm } from 'fs/promises'
import { temporaryDirectory } from 'tempy'
import { handleQuestions, CONFIRM, DOWN, NO, answerWithValue } from '../../utils/handle-questions.js'
import execa from 'execa'
import { cliPath } from '../../utils/cli-path.js'
import { join } from 'path'

const TABTAB_CONFIG_LINE = '[[ -f ~/.config/tabtab/__tabtab.zsh ]] && . ~/.config/tabtab/__tabtab.zsh || true'
const AUTOLOAD_COMPINIT = 'autoload -U compinit; compinit'
Comment on lines +10 to +11
Copy link
Contributor

Choose a reason for hiding this comment

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

pull these from constants when you make the change from above


describe('completion:install command', () => {
let tempDir
let zshConfigPath
let options

beforeAll(() => {
tempDir = temporaryDirectory()
zshConfigPath = join(tempDir, '.zshrc')
options = { cwd: tempDir, env: { HOME: tempDir } }
})

afterAll(async () => {
await rm(tempDir, { force: true, recursive: true })
})

test.skipIf(process.env.SHELL !== '/bin/zsh')(
'should add compinit to .zshrc when user confirms prompt',
async (t) => {
fs.writeFileSync(zshConfigPath, TABTAB_CONFIG_LINE)
const childProcess = execa(cliPath, ['completion:install'], options)

handleQuestions(childProcess, [
{
question: 'Which Shell do you use ?',
answer: answerWithValue(DOWN),
},
{
question: 'We will install completion to ~/.zshrc, is it ok ?',
answer: CONFIRM,
},
{
question: 'Would you like to add it?',
answer: CONFIRM,
},
])

await childProcess
const content = fs.readFileSync(zshConfigPath, 'utf8')
expect(content).toContain(AUTOLOAD_COMPINIT)
},
)

test.skipIf(process.env.SHELL !== '/bin/zsh')(
'should not add compinit to .zshrc when user does not confirm prompt',
async (t) => {
fs.writeFileSync(zshConfigPath, TABTAB_CONFIG_LINE)
const childProcess = execa(cliPath, ['completion:install'], options)

handleQuestions(childProcess, [
{
question: 'Which Shell do you use ?',
answer: answerWithValue(DOWN),
},
{
question: 'We will install completion to ~/.zshrc, is it ok ?',
answer: CONFIRM,
},
{
question: 'Would you like to add it?',
answer: answerWithValue(NO),
},
])

await childProcess
const content = fs.readFileSync(zshConfigPath, 'utf8')
expect(content).not.toContain(AUTOLOAD_COMPINIT)
},
)
})