-
Notifications
You must be signed in to change notification settings - Fork 137
fix hitory file path #1146
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix hitory file path #1146
Conversation
❌ Tests Failed✖️no tests failed ✔️540 tests passed(140 flakes) |
ad57d84
to
f5012f9
Compare
There was a problem hiding this 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 refactors how the history file path is determined, making the default configurable via CONFIG[:history_file]
, preserving legacy ~/.rdbg_history
, and falling back to $XDG_STATE_HOME/rdbg/history
.
- Change default
history_file
config value from"~/.rdbg_history"
tonil
- Rewrite
history_file
method inconsole.rb
to respect config, legacy path, then XDG state directory - Update README to document the new default path
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
lib/debug/console.rb | Revised history_file logic to handle nil , legacy file, and XDG state path |
lib/debug/config.rb | Set default history_file to nil |
README.md | Updated default history_file documentation |
Comments suppressed due to low confidence (1)
lib/debug/console.rb:153
- Add tests for the new
history_file
logic: nil config, legacy~/.rdbg_history
, and XDG state directory fallback.
def history_file
* Set `CONFIG[:history_file]` as nil, and ignore it if it is nil. * Respect `CONFIG[:history_file]` even if the path doesn't exists. * Respect `~/.rdbg_history` if exists for compatibility. * Use `XDG_STATE_HOME` if there is no configuration about history file.
f5012f9
to
4acb6bd
Compare
This is great! Do you think we need to document the fallback when XDG_STATE_HOME is unset? This PR does use the basedir specification's default (yay!), but that behavior isn't documented. Would it be welcome if I open a PR to modify the docs to that effect? |
is written. not enough? |
I think the confusion is for users who have never heard of XDG before and don't have The code itself shows the fallback ( I would suggest using bash's own expansion syntax to indicate the env var's fallback in-line:
This way, they don't need to google
|
CONFIG[:history_file]
as nil, and ignore it if it is nil.CONFIG[:history_file]
even if the path doesn't exists.~/.rdbg_history
if exists for compatibility.XDG_STATE_HOME
if there is no configuration about history file.