Skip to content

Conversation

@1orelai
Copy link

@1orelai 1orelai commented Dec 25, 2025

User description

Renamed banish-cookies function to banish_cookies to fix invalid identifier

https://www.gnu.org/software/bash/manual/bash.html#index-identifier

A word consisting solely of letters, numbers, and underscores, and beginning with a letter or underscore. Names are used as shell variable and function names. Also referred to as an identifier.

Currently, this warning is given when lib/bash.sh is sourced (i.e. when invoking an interactive shell)

~/git/oh-my-bash (master) $ source lib/base.sh
bash: `banish-cookies': not a valid identifier`

PR Type

Bug fix


Description

  • Renamed banish-cookies function to banish_cookies

  • Fixed invalid bash identifier with hyphen character

  • Resolves bash warning when sourcing lib/base.sh


Diagram Walkthrough

flowchart LR
  A["banish-cookies<br/>invalid identifier"] -- "rename to valid identifier" --> B["banish_cookies<br/>valid identifier"]
Loading

File Walkthrough

Relevant files
Bug fix
base.sh
Rename function with hyphen to underscore                               

lib/base.sh

  • Renamed function from banish-cookies to banish_cookies
  • Updated function comment to reflect new name
  • Fixes bash identifier validation error
+2/-2     

@qodo-free-for-open-source-projects

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
Unsafe file deletion

Description: The rm -r command operates on user home directory paths without confirmation or safety
checks, potentially causing unintended data loss if the function is accidentally invoked
or if symbolic links already exist.
base.sh [63-63]

Referred Code
rm -r ~/.macromedia ~/.adobe
ln -s /dev/null ~/.adobe
Ticket Compliance
🎫 No ticket provided
  • Create ticket/issue
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Missing error handling: The rm -r command on line 63 could fail if directories don't exist, and ln -s
commands could fail if links already exist, with no error handling or validation.

Referred Code
rm -r ~/.macromedia ~/.adobe
ln -s /dev/null ~/.adobe
ln -s /dev/null ~/.macromedia

Learn more about managing compliance generic rules or creating your own custom rules

Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-free-for-open-source-projects

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Make file operations more robust

Update the banish_cookies function to use rm -rf and ln -sfn to make the file
and directory operations more robust and prevent potential errors.

lib/base.sh [62-66]

 function banish_cookies {
-  rm -r ~/.macromedia ~/.adobe
-  ln -s /dev/null ~/.adobe
-  ln -s /dev/null ~/.macromedia
+  rm -rf ~/.macromedia ~/.adobe
+  ln -sfn /dev/null ~/.adobe
+  ln -sfn /dev/null ~/.macromedia
 }
  • Apply / Chat
Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies potential failure modes and provides robust fixes, using rm -rf to avoid errors and ln -sfn to ensure the symlink correctly replaces the target, which significantly improves the function's reliability.

Medium
  • More

@akinomyoga
Copy link
Contributor

Currently, this warning is given when lib/bash.sh is sourced (i.e. when invoking an interactive shell)

~/git/oh-my-bash (master) $ source lib/base.sh
bash: `banish-cookies': not a valid identifier`

That implies that you turn on the POSIX mode of Bash (by either setting the shell option -o posix, setting an environment variable POSIXLY_CORRECT, or starting Bash with the name sh). However, Oh My Bash hasn't been caring about the POSIX mode, and it's not surprising if it is broken in various places, not only the warning message for lib/base.sh.

@akinomyoga
Copy link
Contributor

A word consisting solely of letters, numbers, and underscores, and beginning with a letter or underscore. Names are used as shell variable and function names. Also referred to as an identifier.

Please also notice the description in 3.3 Shell Functions - Bash Reference Manual:

When not in POSIX mode, a function name can be any unquoted shell word that does not contain ‘$’.

@1orelai
Copy link
Author

1orelai commented Dec 25, 2025

@akinomyoga I'm typically using OMB on Linux, only noticed this on a fresh installation of MacOS

Macports and Homebrew both appear to use --posix by default to start Bash 4+ (i.e. not /bin/bash built-in) I can disable this manually, but POSIXLY_CORRECT is defined in the environment by default, causing a warning on start-up after installing OMB

Besides this function, everything appears to be working as expected when --posix is set

I understand why OMB isn't written for POSIX compliance, but the warning seems avoidable? If the mode should be avoided entirely, why not explicitly handle this? e.g. [[ -v POSIXLY_CORRECT ]]

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants