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

Honor Pry.config.should_load_local_rc = false in global RC file #2243

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ajvondrak
Copy link

The Pry.config.should_load_local_rc flag was first added for #612. The
intended use case was to set it to false in your "global" RC file
(~/.pryrc, ~/.config/pry/pryrc, etc) to disable local RC files.

However, the refactoring in 78caffb
broke this functionality, because Pry.rc_files_to_load evaluates
Pry.config.should_load_local_rc before we ever load anything via
Pry.load_rc_files. The only way should_load_local_rc gets disabled
before this point is if we pass in the -f flag to Pry, which disables
all RC files.

The main point of Pry.rc_files_to_load is that it resolves the RC
paths and de-duplicates them, in case the global one symlinks to the
local one or vice versa. But it attempts to do this "ahead of time",
generating the list of files first before we ever load any of them, then
calling Array#uniq. This dedups the paths, but populating the array in
the first place means we check the should_load_local_rc boolean before
the global RC ever has a chance to set it.

This PR undoes some of that refactoring, bringing it closer to the old
code that had separate steps for Pry.load_rc and Pry.load_local_rc,
so we can check the config flag after the side effect of loading the
global RC file. However, it maintains the more general interface of
Pry.load_rc_files, in case other types of RC files are added in the
future. No matter the type of RC file, they all funnel through
Pry.load_rc_file, which now handles resolving paths and preventing
duplicates. It does this by maintaining Pry.rc_files_loaded, shifting
the logic from the more static "what will we load in the future?" to the
more dynamic "what have we loaded in the past?"

The Pry.rc_files_to_load method is removed by this PR, since it's not
necessarily an accurate reflection of the files that ultimately do get
loaded. Because it was public, this is technically a breaking change to
the API. However, note that the interface of Pry.load_rc_files remains
intact: true to the docstring, you can reload the RC files because we
take care to clear the Pry.rc_files_loaded array at the start.

The `Pry.config.should_load_local_rc` flag was first added for pry#612. The
intended use case was to set it to `false` in your "global" RC file
(~/.pryrc, ~/.config/pry/pryrc, etc) to disable local RC files.

However, the refactoring in 78caffb
broke this functionality, because `Pry.rc_files_to_load` evaluates
`Pry.config.should_load_local_rc` before we ever load anything via
`Pry.load_rc_files`. The only way `should_load_local_rc` gets disabled
before this point is if we pass in the `-f` flag to Pry, which disables
*all* RC files.

The main point of `Pry.rc_files_to_load` is that it resolves the RC
paths and de-duplicates them, in case the global one symlinks to the
local one or vice versa. But it attempts to do this "ahead of time",
generating the list of files first before we ever load any of them, then
calling `Array#uniq`. This dedups the paths, but populating the array in
the first place means we check the `should_load_local_rc` boolean before
the global RC ever has a chance to set it.

This PR undoes some of that refactoring, bringing it closer to the old
code that had separate steps for `Pry.load_rc` and `Pry.load_local_rc`,
so we can check the config flag after the side effect of loading the
global RC file. However, it maintains the more general interface of
`Pry.load_rc_files`, in case other types of RC files are added in the
future. No matter the type of RC file, they all funnel through
`Pry.load_rc_file`, which now handles resolving paths and preventing
duplicates. It does this by maintaining `Pry.rc_files_loaded`, shifting
the logic from the more static "what will we load in the future?" to the
more dynamic "what have we loaded in the past?"

The `Pry.rc_files_to_load` method is removed by this PR, since it's not
necessarily an accurate reflection of the files that ultimately do get
loaded. Because it was public, this is technically a breaking change to
the API. However, note that the interface of `Pry.load_rc_files` remains
intact: true to the docstring, you can reload the RC files because we
take care to clear the `Pry.rc_files_loaded` array at the start.
kg8m added a commit to kg8m/dotfiles that referenced this pull request May 6, 2022
Note: currently the `should_load_local_rc` option doesn't work.
cf. pry/pry#2243
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.

None yet

1 participant