Skip to content

Fix _purge_line when noclobber is enabled#88

Closed
jazz-it wants to merge 3 commits into
huyng:masterfrom
jazz-it:fix-noclobber-support
Closed

Fix _purge_line when noclobber is enabled#88
jazz-it wants to merge 3 commits into
huyng:masterfrom
jazz-it:fix-noclobber-support

Conversation

@jazz-it

@jazz-it jazz-it commented Jul 1, 2026

Copy link
Copy Markdown

Summary

This PR fixes a failure in _purge_line when Bash is configured with set -o noclobber (or set -C).

The previous implementation could fail while updating the bookmarks file because of the way the temporary file was created and written. As a result, commands that remove or update bookmarks would not work correctly in shells with noclobber enabled.

Changes

  • Simplified _purge_line.
  • Replaced the previous sed-based temporary file workflow with a grep -v filter.
  • Continue using mktemp for secure temporary file creation before replacing the original file.

Why

Many users enable noclobber in their .bashrc or .bash_profile to prevent accidental file overwrites. This change makes bashmarks behave correctly in that configuration without requiring users to disable the option.

Testing

Tested by enabling:

set -o noclobber

and verifying that bookmark removal/update operations complete successfully.

@huyng

huyng commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Thanks for raising this issue again and for submission. I ended up using using the solution proposed here: https://github.com/huyng/bashmarks/pull/66/changes

@huyng huyng closed this Jul 5, 2026
@jazz-it

jazz-it commented Jul 5, 2026

Copy link
Copy Markdown
Author

Thanks for reviewing the patch.
The >| operator is correct in principle, but unfortunately it does not work reliably when noclobber is enabled before the function is parsed - especially in .bashrc‑loaded functions and on systems where mktemp creates a regular file.
In these cases Bash treats >| as a normal > and still blocks the redirect.
This affects Bash 3.x/4.x/5.x on both GNU/Linux and macOS.

The portable fix I proposed here avoids redirects entirely and works across GNU/BSD sed, BusyBox, and all shells (bash, zsh, dash, ash), regardless of when noclobber is enabled.
It also avoids mktemp‑related edge cases and parsing differences between interactive and non‑interactive shells.

Because of that, the grep‑based version is more robust and covers additional real‑world scenarios that >| cannot handle.

Hopefully you will consider re‑opening this PR, as the portable fix handles noclobber and mktemp redirect edge cases that >| does not.

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