Skip to content

fix: match typeset in readonly guard for zsh compatibility#115

Merged
GingerGraham merged 1 commit into
GingerGraham:mainfrom
Jah-yee:fix/zsh-readonly-variable
May 27, 2026
Merged

fix: match typeset in readonly guard for zsh compatibility#115
GingerGraham merged 1 commit into
GingerGraham:mainfrom
Jah-yee:fix/zsh-readonly-variable

Conversation

@Jah-yee
Copy link
Copy Markdown
Contributor

@Jah-yee Jah-yee commented May 26, 2026

Problem

When sourcing bash-logger from .zshrc, the re-sourcing guard on line 57 fails because readonly -p in zsh outputs typeset instead of declare. This causes:

logging.sh:57: read-only variable: BASH_LOGGER_VERSION

Fix

Extend the grep pattern to match both declare and typeset:

- if ! readonly -p 2>/dev/null | grep -q "declare -[^ ]*r[^ ]* BASH_LOGGER_VERSION=";
+ if ! readonly -p 2>/dev/null | grep -qE '(declare|typeset) -[^ ]*r[^ ]* BASH_LOGGER_VERSION=';

Testing

Reported and confirmed by the original issue author (issue #112).

Closes #112

In zsh, readonly variables are reported with 'typeset' instead of
'declare', causing the re-sourcing guard to miss the variable and
attempt to redeclare it, triggering 'read-only variable' errors.

Closes GingerGraham#112
@Jah-yee Jah-yee requested a review from GingerGraham as a code owner May 26, 2026 18:45
@GingerGraham
Copy link
Copy Markdown
Owner

@Jah-yee thank you for running with this. Any chance that you could clean up the PR comments though to remove all the OpenClaw inserted text and better describe the submitted change?

Not that I'm against using AI tooling as you can tell from my own code, but I am pro- human readable transparency and so I'd prefer PRs to be readable as to the issue it's addressing, the fix/changes and testing to validate.

I've mocked up the solution myself as I was opening the issue so I'm confident it's good, but having a second set of test eyes on it from the contributor would be preferrable.

Thanks again!

@GingerGraham
Copy link
Copy Markdown
Owner

Tests and styling all good, locally testing shows OK too.

Just need to clean up the PR itself and then this one is good to go

Copy link
Copy Markdown
Owner

@GingerGraham GingerGraham left a comment

Choose a reason for hiding this comment

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

Change in line with what I was expecting

@Jah-yee
Copy link
Copy Markdown
Contributor Author

Jah-yee commented May 26, 2026

Done! PR description has been cleaned up — removed all the tooling noise and kept just the clean Problem/Fix/Testing format. Thanks for the feedback, noted for future PRs.

@GingerGraham GingerGraham merged commit 412e766 into GingerGraham:main May 27, 2026
2 checks passed
GingerGraham pushed a commit that referenced this pull request May 27, 2026
## [2.5.2](2.5.1...2.5.2) (2026-05-27)

### Bug Fixes

* match typeset in readonly guard for zsh compatibility ([#115](#115))

### Documentation

* add claude skills for linting, pre-PR checks, running tests, se… ([#109](#109))
* establish AGENTS.md as canonical agent instruction file ([#110](#110))
@GingerGraham
Copy link
Copy Markdown
Owner

🎉 This PR is included in version 2.5.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] read-only variable: BASH_LOGGER_VERSION output when sourcing with zsh

2 participants