Skip to content

Conversation

@hutstep
Copy link
Contributor

@hutstep hutstep commented Nov 11, 2025

No description provided.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR focuses on dependency updates, refactoring, and improvements to Unicode/multibyte character handling throughout the application. The changes modernize terminal interaction, improve error handling with proper file closure patterns, and enhance string display for international characters.

  • Updates Go dependencies and toolchain to newer versions
  • Refactors string width calculations to properly handle multibyte characters (e.g., full-width Asian characters)
  • Improves error handling with deferred file closures and formatted log messages
  • Replaces terminal size detection library from olekukonko/ts to golang.org/x/term

Reviewed Changes

Copilot reviewed 17 out of 19 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
go.mod Updates Go version and dependencies; adds mattn/go-runewidth and golang.org/x/term
go.sum Reflects updated dependency checksums
utils/strings.go Changes string length calculation to use visual width instead of byte length
utils/cobra.go Updates error logging to use format specifiers
prompt/pad.go Adds new pad function for Unicode-aware string padding and truncation
prompt/pad_test.go Adds comprehensive tests for the new pad function
prompt/templates.go Refactors templates to use new pad function instead of trunc + print
prompt/prompt.go Replaces olekukonko/ts with golang.org/x/term for terminal size detection; adds pad to template functions
cmd/login.go Improves log formatting with format specifiers
azurecli/internal.go Adds deferred file closing with error checking; updates error messages
azurecli/azurecli.go Adds error handling for spinner color configuration
.goreleaser.yaml Changes from brews to homebrew_casks configuration
.goreleaser-windows.yaml Removes ARM 32-bit support for Windows
.golangci.yml Updates to golangci-lint v2 configuration format
.github/workflows/*.yaml Updates Go version and GitHub Actions versions
.gitignore Adds .vscode/ and AGENTS.md to ignored files
output.txt, output2.txt Test output files (likely should not be committed)

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +37 to +113
settings:
dupl:
# Tokens count to trigger issue.
# Default: 150
threshold: 100

linters-settings:
dupl:
# Tokens count to trigger issue.
# Default: 150
threshold: 100

errorlint:
# Check whether fmt.Errorf uses the %w verb for formatting errors.
# Default: true
asserts: false
errorlint:
# Check whether fmt.Errorf uses the %w verb for formatting errors.
# Default: true
asserts: false

funlen:
# Checks the number of lines in a function.
# Default: 60
lines: -1
# Default: 40
statements: 50
# Ignore comments when counting lines.
# Default false
ignore-comments: true
funlen:
# Checks the number of lines in a function.
# Default: 60
lines: -1
# Default: 40
statements: 50
# Ignore comments when counting lines.
# Default false
ignore-comments: true

goconst:
# Minimal length of string constant.
# Default: 3
min-len: 2
# Default: 3
min-occurrences: 3
goconst:
# Minimal length of string constant.
# Default: 3
min-len: 2
# Default: 3
min-occurrences: 3

# gocritic:
# enabled-tags:
# - diagnostic
# - experimental
# - opinionated
# - performance
# - style
# disabled-checks:
# - dupImport # https://github.com/go-critic/go-critic/issues/845
# - ifElseChain
# - octalLiteral
# - whyNoLint
# gocritic:
# enabled-tags:
# - diagnostic
# - experimental
# - opinionated
# - performance
# - style
# disabled-checks:
# - dupImport # https://github.com/go-critic/go-critic/issues/845
# - ifElseChain
# - octalLiteral
# - whyNoLint

gocyclo:
# Minimal code complexity to report.
# Default: 30 (but we recommend 10-20)
min-complexity: 15
gocyclo:
# Minimal code complexity to report.
# Default: 30 (but we recommend 10-20)
min-complexity: 15

godox:
keywords:
- FIXME
#- TODO
- FIX
- NOTE
- OPTIMIZE # marks code that should be optimized before merging
- HACK # marks hack-around that should be removed before merging
godox:
keywords:
- FIXME
#- TODO
- FIX
- NOTE
- OPTIMIZE # marks code that should be optimized before merging
- HACK # marks hack-around that should be removed before merging

gofmt:
# Simplify code: gofmt with `-s` option.
# Default: true
simplify: false
# Apply the rewrite rules to the source before reformatting.
# https://pkg.go.dev/cmd/gofmt
# Default: []
rewrite-rules:
# - pattern: 'interface{}'
# replacement: 'any'
- pattern: 'a[b:len(a)]'
replacement: 'a[b:]'
# govet:
# enable-all: true
# disable:
# - fieldalignment

goimports:
local-prefixes: github.com/golangci/golangci-lint
misspell:
# Correct spellings using locale preferences for US or UK.
locale: US

# govet:
# enable-all: true
# disable:
# - fieldalignment
nolintlint:
# Disable to ensure that all nolint directives actually have an effect.
allow-unused: false # report any unused nolint directives
require-explanation: true # require an explanation for nolint directives
require-specific: true # require nolint directives to be specific about which linter is being skipped

misspell:
# Correct spellings using locale preferences for US or UK.
locale: US
# revive:
# rules:
# - name: indent-error-flow
# - name: unexported-return
# disabled: true
# - name: unused-parameter
# - name: unused-receiver
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

The settings key should be nested under linters, not at the same level. The correct structure for golangci-lint v2 is linters.settings not linters as a separate top-level key alongside settings. This configuration may not work as intended.

Copilot uses AI. Check for mistakes.
// Verify that the config dir exists
if !utils.FileExists(configDir) {
return "", fmt.Errorf("%s (%s) is not a valid directory. Please run `az configure` and try again.", CONFIG_DIR_ENV, configDir)
return "", fmt.Errorf("%s (%s) is not a valid directory; run `az configure` and try again", CONFIG_DIR_ENV, configDir)
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

The error message was simplified but removed important context. The original message "Please run az configure and try again." provided clear guidance. The new message "run az configure and try again" is less polite and slightly more terse. Consider keeping the "Please" for better user experience.

Suggested change
return "", fmt.Errorf("%s (%s) is not a valid directory; run `az configure` and try again", CONFIG_DIR_ENV, configDir)
return "", fmt.Errorf("%s (%s) is not a valid directory; Please run `az configure` and try again.", CONFIG_DIR_ENV, configDir)

Copilot uses AI. Check for mistakes.
configFilePath := fmt.Sprintf("%s/%s", configDir, PROFILES_JSON)
if !utils.FileExists(configFilePath) {
return fmt.Errorf("%s is not a valid file. Please run `az configure` and try again.", configFilePath)
return fmt.Errorf("%s is not a valid file; run `az configure` and try again", configFilePath)
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

The error message was simplified but removed important context. The original message "Please run az configure and try again." provided clear guidance. The new message "run az configure and try again" is less polite and slightly more terse. Consider keeping the "Please" for better user experience.

Suggested change
return fmt.Errorf("%s is not a valid file; run `az configure` and try again", configFilePath)
return fmt.Errorf("%s is not a valid file; Please run `az configure` and try again", configFilePath)

Copilot uses AI. Check for mistakes.
Comment on lines +115 to +132
formatters:
enable:
- gofmt
- gofumpt
- goimports
settings:
gofmt:
# Simplify code: gofmt with `-s` option.
# Default: true
simplify: false
# Apply the rewrite rules to the source before reformatting.
# https://pkg.go.dev/cmd/gofmt
# Default: []
rewrite-rules:
# - pattern: 'interface{}'
# replacement: 'any'
- pattern: "a[b:len(a)]"
replacement: "a[b:]"
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

The formatters section is not a standard golangci-lint v2 configuration key. Formatters like gofmt, gofumpt, and goimports should be enabled in the linters.enable section, not in a separate formatters section. This configuration will likely be ignored.

Copilot uses AI. Check for mistakes.
Comment on lines +53 to +73
homebrew_casks:
- name: azctx
repository:
owner: whiteducksoftware
name: homebrew-tap
branch: main
token: "{{ .Env.AUTH_GITHUB }}"
dependencies:
- azure-cli
url_template: "https://github.com/whiteducksoftware/azctx/releases/download/{{ .Tag }}/{{ .ArtifactName }}"
commit_author:
name: "{{ .Env.GITHUB_ACTOR }}"
email: "{{ .Env.GITHUB_ACTOR_ID }}+{{ .Env.GITHUB_ACTOR }}@users.noreply.github.com"
commit_msg_template: "Brew formula update for {{ .ProjectName }} version {{ .Tag }}"
directory: Formula
binaries:
- azctx
directory: Casks
homepage: "https://github.com/whiteducksoftware/azctx"
description: "Faster switching between Azure Subscriptions in your Azure CLI"
license: "MIT"
install: |
bin.install "azctx"
commit_author:
name: "{{ .Env.GITHUB_ACTOR }}"
email: "{{ .Env.GITHUB_ACTOR_ID }}+{{ .Env.GITHUB_ACTOR }}@users.noreply.github.com"
commit_msg_template: "Brew cask update for {{ .ProjectName }} version {{ .Tag }}"
url:
template: "https://github.com/whiteducksoftware/azctx/releases/download/{{ .Tag }}/{{ .ArtifactName }}"
dependencies:
- formula: azure-cli
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

The configuration key changed from brews to homebrew_casks, but the configuration appears to be for a Homebrew formula (command-line tool), not a cask (GUI application). Homebrew casks are typically for GUI applications distributed as .app bundles, .dmg files, or .pkg installers. For a CLI tool like azctx, this should likely remain as brews (or the v2 equivalent homebrew), not homebrew_casks.

Copilot uses AI. Check for mistakes.
Comment on lines +25 to +29
if width, height, err := term.GetSize(int(os.Stdout.Fd())); err != nil {
log.Warn("Unable to get terminal dimensions, using default values (width: %d, height: %d)", terminalWidth, terminalHeigth)
} else {
// Set the terminal dimensions
terminalWidth = size.Col()
terminalHeigth = size.Row()
terminalWidth = width
terminalHeigth = height
Copy link

Copilot AI Nov 12, 2025

Choose a reason for hiding this comment

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

The error handling logic is inverted. The condition should check err == nil (successful call) to use the returned width and height, not err != nil (error case). Currently, when term.GetSize() succeeds, the default values (100, 20) are used instead of the actual terminal dimensions.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants