Skip to content

Conversation

chgl
Copy link
Contributor

@chgl chgl commented Aug 22, 2025

Closes #

Description

I just couldn't get the script to execute properly in its current form. I saw e.g.

[[: 1989{#d[@]}: syntax error: invalid arithmetic operator (error token is "{#d[@]}")

when trying to run the script locally. (GNU bash, version 5.2.21(1)-release (x86_64-pc-linux-gnu)).

This uses a likely simpler bash script, but requires both grep and awk.

Type of Change

  • New module
  • Bug fix
  • Feature/enhancement
  • Documentation
  • Other

Module Information

Path: registry/coder/modules/kasmvnc
New version: v1.2.3
Breaking change: [ ] Yes [x] No

Testing & Validation

  • Tests pass (bun test)
  • Code formatted (bun run fmt)
  • Changes tested locally

Related Issues

@chgl chgl marked this pull request as ready for review August 22, 2025 13:38
@matifali matifali requested a review from angrycub August 23, 2025 17:40
@matifali matifali added the version:patch Add to PRs requiring a patch version upgrade label Aug 23, 2025
@matifali matifali requested a review from Copilot August 23, 2025 20:48
Copy link
Contributor

@Copilot 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 fixes a bash syntax error in the get_http_dir() function that was preventing the script from executing properly on certain bash versions. The original code used bash array syntax that was causing parsing errors.

  • Simplified the bash script logic by replacing array-based parsing with grep and awk
  • Updated module version from 1.2.2 to 1.2.3

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
registry/coder/modules/kasmvnc/run.sh Refactored get_http_dir function to use grep+awk instead of bash arrays
registry/coder/modules/kasmvnc/README.md Updated version number to reflect the bug fix

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

chgl and others added 4 commits August 26, 2025 19:31
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
version:patch Add to PRs requiring a patch version upgrade
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants