fix(config): enable file include in scoped queries - #2345
Conversation
|
@becm Thanks for identifying this key piece that connects to several issues. Could you add a test to demonstrate the behavior change to prevent regressions in the future? |
|
@derrickstolee sadly only the 1st step, since:
|
|
@derrickstolee since the Will try to implement a test for (abstract) config data with the (hopefully) resolved corner case of |
|
Reverted PR to primary goal, will tackle @derrickstolee I'll try to add a test for file inclusion (global scope). |
This comment was marked as spam.
This comment was marked as spam.
|
@mjcheetham any further actions to be taken to accept this? This change will essentially harmonizes behavior before and after |
|
@derrickstolee it seems Would it be possible to introduce a further code path for This would be an extension or (for non-ancient Git versions) a replacement for the current PR. |
|
The way I read this change, it constitutes a change of behavior. A good one, granted, but one that has a high potential of affecting existing users' setups (especially complex setups). As such, I'd be much more comfortable to make this behavioral change part of a major version bump. Seeing as v3.x is starting to take concrete shape, I would consider that version to be the natural inflection point to integrate this here PR. |
|
@dscho behavior change already happened for This combination effectively uses |
@becm sure, but the way I see it was that this was an inadvertent behavioral change. Here, we are aware of the change. That makes a huge difference. |
|
@dscho the problem is inconsistent behavior when different Git versions are involved. So a "bugfix" is to intentionally lean into the (unintended) change. If adding config cache did not enforce |
|
Now that |
|
If there is serious intent to merge, I'd invest work to submit a better (more extensive) approach.
Only using The approach using |
test special conditional global include based on local Git remote value verify unconditional include directive for local scope
split configuration cache checks for raw and typed values drop `--no-type` for Raw values (default) to support older Git versions
match query results of `ConfigCache` as close as possible
|
Revised for
|
|
@becm thank you for rebasing! @mjcheetham and I should be able to focus on getting this merged this coming week. |
Avoid duplication of entries already present in included files:
Rawvalues in all Git versions supporting--show-scopeImproves consistency regarding include behavior and
Fixes #2342
Fixes #2328
Fixes #1696
Fixes #1337