You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Feature request: I'd like to be able to supply multiple special_commands entries with overlapping globs and all that match become available together in the file command popup.
e.g., given some config like:
[special_commands]
"*" = [
"echo make this available for anything!"
]
"*.rs" = [
"echo this command is valid on any rust file"
]
"specifically_me.rs" = [
"echo literals are still fine also",
]
"src/**/test_*.rs" = [
"echo overly fancy demo of globs"
]
... files in this projectable should have between one and four special commands depending on how many of these patterns they match!
(And I'm maybe willing to try to write patches for this, so, some code reading notes I made come below too :) But I still want to check on direction before I go further because I think this one might get nontrivial.)
currently
It's already possible to specify many special_commands easily, and use a wide variety of globs. Neato!
However, the behavior of this is not so useful. Currently, if more than one glob matches, you'll the command list for one of those matching globs -- but there's no merging of lists from any other matching globs... and which list you end up with is undefined. (In the example above: for a file "foo.rs", you'll get the list from "*", or from "*.rs", and it's really not defined which one.)
peeking at the code
I did a code splunk to try to find where the current behavior is located:
It appears that the config uses a HashMap for these (that's pt1 of how we end up with a random outcome)...
The randomization of order is happens during FileCmdPopup.new: it uses an iter on the hashmap as it is building the registry vec. (Multiple patterns are still getting passed down, but this is where the shuffle gets laid in.)
The patterns get passed down pretty deeply: They're persisted all the way into FileCmdPopup.registry.
The reduction down to one match happens in FileCmdPopup.open_for, which uses position to pick the first match. (Which one is first was defined earlier, during the iter.)
So there's two routes to fixing the randomization: an IndexMap would do it; so would a sort in FileCmdPopup.new. (Slightly different consequences. I'd probably lean towards using the IndexMap approach, because I like stable behaviors based on user input order more than I like sorting, just as a philosophy -- but that's a personal preference and not strongly held.)
Getting the merge of command lists for all patterns that match... looks a bit more complicated. I confess I'm a tad intimidated by some of the code in that area. Replacing position with something that yields a vec instead of a single choice would be the first step, but there's some complicated stateful interactions with self.opened, and... well, I think I might wanna chat about that code before trying to touch it.
how does this relate to inheritance of multiple config files?
Okay, so, above, I was focusing on when multiple globs match. There's also the question of how multiple configurations get merged. tl;dr: there might be a whole 'nother issue-worth of discussion for that, and I don't know if it's necessary to try to handle it at the same time. But I started thinking about it!, so here goes:
Right now, the special_commands value already gets merged such that if you have multiple config files and they have distinctive pattern strings, those accumulate. That's probably the correct behavior.
If you the same pattern string in a deeper config file, though, it'll overwrite the whole list from the parent config. It doesn't append. This might not be the ideal behavior. I can think of user stories where I want to attach more commands to a glob like "*" as I accumulate .projectable.toml config files, without losing the existing global ones. Making this happen would be a pretty small code change.
I could see someone wanting to drop some global special commands as they get into more detailed config, too, though. I don't know how that should be represented. I think the schema would have to be extended a bit to make that kind of feature possible.
Overall, I'm not sure what changes to make to the multiple config file story here, and it's possible there's some decisions that merit discussion and planning. But working on the multi-matches stuff can probably go ahead either way; they're pretty fairly unentangled.
The text was updated successfully, but these errors were encountered:
Feature request: I'd like to be able to supply multiple
special_commands
entries with overlapping globs and all that match become available together in the file command popup.e.g., given some config like:
... files in this projectable should have between one and four special commands depending on how many of these patterns they match!
(And I'm maybe willing to try to write patches for this, so, some code reading notes I made come below too :) But I still want to check on direction before I go further because I think this one might get nontrivial.)
currently
It's already possible to specify many
special_commands
easily, and use a wide variety of globs. Neato!However, the behavior of this is not so useful. Currently, if more than one glob matches, you'll the command list for one of those matching globs -- but there's no merging of lists from any other matching globs... and which list you end up with is undefined. (In the example above: for a file "foo.rs", you'll get the list from
"*"
, or from"*.rs"
, and it's really not defined which one.)peeking at the code
I did a code splunk to try to find where the current behavior is located:
HashMap
for these (that's pt1 of how we end up with a random outcome)...FileCmdPopup.new
: it uses aniter
on the hashmap as it is building theregistry
vec. (Multiple patterns are still getting passed down, but this is where the shuffle gets laid in.)FileCmdPopup.registry
.FileCmdPopup.open_for
, which usesposition
to pick the first match. (Which one is first was defined earlier, during theiter
.)So there's two routes to fixing the randomization: an
IndexMap
would do it; so would a sort inFileCmdPopup.new
. (Slightly different consequences. I'd probably lean towards using theIndexMap
approach, because I like stable behaviors based on user input order more than I like sorting, just as a philosophy -- but that's a personal preference and not strongly held.)Getting the merge of command lists for all patterns that match... looks a bit more complicated. I confess I'm a tad intimidated by some of the code in that area. Replacing
position
with something that yields a vec instead of a single choice would be the first step, but there's some complicated stateful interactions withself.opened
, and... well, I think I might wanna chat about that code before trying to touch it.how does this relate to inheritance of multiple config files?
Okay, so, above, I was focusing on when multiple globs match. There's also the question of how multiple configurations get merged. tl;dr: there might be a whole 'nother issue-worth of discussion for that, and I don't know if it's necessary to try to handle it at the same time. But I started thinking about it!, so here goes:
Right now, the
special_commands
value already gets merged such that if you have multiple config files and they have distinctive pattern strings, those accumulate. That's probably the correct behavior.If you the same pattern string in a deeper config file, though, it'll overwrite the whole list from the parent config. It doesn't append. This might not be the ideal behavior. I can think of user stories where I want to attach more commands to a glob like
"*"
as I accumulate.projectable.toml
config files, without losing the existing global ones. Making this happen would be a pretty small code change.I could see someone wanting to drop some global special commands as they get into more detailed config, too, though. I don't know how that should be represented. I think the schema would have to be extended a bit to make that kind of feature possible.
Overall, I'm not sure what changes to make to the multiple config file story here, and it's possible there's some decisions that merit discussion and planning. But working on the multi-matches stuff can probably go ahead either way; they're pretty fairly unentangled.
The text was updated successfully, but these errors were encountered: