Skip to content
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

Support XDG directories for config and history files #1055

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

jgarber623
Copy link

Description

Addresses #1031 by implementing support for XDG directories in accordance with the rules discussed in the issue and summarized in my comment here.

This change should be non-breaking in that the code continues to prefer existing default configuration files at ~/.rdbgrc and ~/.rdbg_history.

If those files do not exist and XDG environment variables are configured, then configuration and history files in the relevant XDG directories are used.

  • $XDG_CONFIG_HOME/rdbg/config (if file is present)
  • $XDG_DATA_HOME/rdbg/history

Note that $XDG_CONFIG_HOME is typically $HOME/.config and that $XDG_DATA_HOME is typically $HOME/.local/share. These are customizable by users, but I believe those are the customary/default paths.

This commit implements XDG directory support for this gem's history file
in accordance with the rules outlined in ruby#1031:

> For the history file:
>
> 1. prefer `~/.rdbg_history` if present,
> 2. else, `$XDG_DATA_HOME/rdbg/history` if `$XDG_DATA_HOME` is set¹
>
> ¹ There'd need to be a check for this file path. If it exists, great!
> If not, create the path `$XDG_DATA_HOME/rdbg` and touch
> `$XDG_DATA_HOME/rdbg/history`.

See: ruby#1031 (comment)
This commit implements XDG directory support for this gem's `.rdbgrc`
configuration file in accordance with the rules outlined in ruby#1031:

> 1. prefer `~/.rdbgrc` if present,
> 2. else, `$XDG_CONFIG_HOME/rdbg/config` if `$XDG_CONFIG_HOME` is set
> and `$XDG_CONFIG_HOME/rdbg/config` is present,
> 3. else, no customized user configuration

See: ruby#1031 (comment)
@jgarber623 jgarber623 marked this pull request as ready for review January 30, 2024 21:19
]

if (xdg_home = ENV["XDG_CONFIG_HOME"])
rc_file_paths << [File.expand_path(File.join(xdg_home, "rdbg", "config")), true]
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't you need config.rb here?

Copy link
Author

Choose a reason for hiding this comment

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

Good catch. This change should support both ${XDG_CONFIG_HOME}/rdbg/config and ${XDG_CONFIG_HOME}/rdbg/config.rb. I'll make that update.

Copy link
Author

Choose a reason for hiding this comment

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

@ko1 Thank you again for the feedback. I've added support for config.rb in this path in 55f6624.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you!

Copy link

launchable-app bot commented Mar 29, 2024

Launchable Report

❌ Test session #2769253 failedos:ubuntu-latest test_task:test_consoledetails on CI
🔔 no issues ✖️118 tests failed ✔️179 tests passed

❌ Test session #2769258 failedos:ubuntu-latest test_task:test_consoledetails on CI
🔔 no issues ✖️121 tests failed ✔️176 tests passed

❌ Test session #2769260 failedos:ubuntu-latest test_task:test_consoledetails on CI
🔔 no issues ✖️118 tests failed ✔️179 tests passed

Passed test sessions

✅ Test session #2769168 passed os:ubuntu-latest test_task:test_testdetails on CI
✅ Test session #2769169 passed os:ubuntu-latest test_task:test_testdetails on CI
✅ Test session #2769171 passed os:ubuntu-latest test_task:test_testdetails on CI
✅ Test session #2769172 passed os:ubuntu-latest test_task:test_testdetails on CI
✅ Test session #2769176 passed os:ubuntu-latest test_task:test_testdetails on CI
✅ Test session #2769180 passed os:ubuntu-latest test_task:test_testdetails on CI
✅ Test session #2769182 passed os:ubuntu-latest test_task:test_protocoldetails on CI
✅ Test session #2769183 passed os:ubuntu-latest test_task:test_protocoldetails on CI
✅ Test session #2769184 passed os:ubuntu-latest test_task:test_protocoldetails on CI
✅ Test session #2769185 passed os:ubuntu-latest test_task:test_protocoldetails on CI
✅ Test session #2769186 passed os:ubuntu-latest test_task:test_protocoldetails on CI
✅ Test session #2769189 passed os:ubuntu-latest test_task:test_protocoldetails on CI
✅ Test session #2769201 passed os:ubuntu-latest test_task:test_protocoldetails on CI
✅ Test session #2769215 passed os:macos-latest test_task:test_consoledetails on CI
✅ Test session #2769220 passed os:macos-latest test_task:test_consoledetails on CI
✅ Test session #2769222 passed os:ubuntu-latest test_task:test_consoledetails on CI
✅ Test session #2769223 passed os:ubuntu-latest test_task:test_consoledetails on CI
✅ Test session #2769224 passed os:ubuntu-latest test_task:test_consoledetails on CI
✅ Test session #2769240 passed os:macos-latest test_task:test_consoledetails on CI
✅ Test session #2769257 passed os:ubuntu-latest test_task:test_consoledetails on CI

Build: refs_pull_1055_merge_55f66240abf152a91a42df8aa4d05e2a25387bdd

@jgarber623
Copy link
Author

Hello! I don't have the ability to re-run the failed checks and it's not immediately clear to me why those failed (and why Ruby 3.2 and 3.3 passed).

For someone more knowledgeable, do those failures look to be caused by changes in this pull request? Should I update this branch with a merge commit and have someone re-run the test suite?

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

Successfully merging this pull request may close these issues.

2 participants