-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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 per-repo config files #3293
Comments
In practice, I think option 2 is preferable. Yes, If a location is needed outside the worktree, e.g. because the dev wants to make a per-repo setting but doesn’t have control of
That said, I’d leave that off entirely lacking a concrete user need/use case to drive it since I think just versioning repo-local Beyond that, I do agree that this is overall a great feature, esp. re: how you note it relates to |
That's an assumption that may be true for some of the examples mentioned above, but maybe not for others (e.g. the This makes me think that we should actually support both locations, so that users can have both versioned configs and personal ones. And the versioned one should probably be read first so that users can override it with their private one.
I'd be pretty strongly against that, since paths or even repo names can change and things become stale. Git itself uses this approach for keeping track of which repos are under |
Please correct me if there's a better way, but I think adding the local config to This feature does sound great; I would use it a lot for commit prefixes. What about the possibility of having a |
Good example, point taken.
Agreed. Likewise, I think my idea is moot in light of supporting a hierarchy involving both |
Completely true. I even have a git helper script to expedite this use case:
Users can already provide their chosen defaults in |
@stefanhaller I just noted that lazygit config can specify full user commands. That suggests that recognized |
Yes I agree w/ that in terms of discoverability. I had no idea what
I think I failed to explain my envisioned use-case. I have my global defaults, but I'm contributing to a repository which has a different commit prefix convention than I normally like to use. I want to quickly set up a config for my clone of that repo because I don't want to add those prefixes to my global config. Or if I'm working with a couple different teams and Team 1 has one convention, Team 2 has a different convention. I don't want to have too many prefix presets in my global config, and I don't want to have to try and remember which prefixes belong to which team if I see them all in a list together. If neither team consents to a tracked So I think what I was intending was more something that helps automate setting up a local config in a repo which doesn't already have one. I totally agree, my idea has no utility in a situation where the repo you're working in already has a tracked I think if lazygit adds the option to have a local config (and I think it should), it would be nice if it could help you stub one out when you need one instead of making the user go copy/paste it out of the docs. |
Yes, this was brought up in the mailing list thread I cited above. It was careless of me not to mention it in the PR description, I added it now. Unfortunately this would make the implementation a lot less straightforward. I had hoped we could implement the feature by just adding a few lines of code to unmarshal the local config file into the config struct after the global one. |
It seems to me like there may only be a few options that make sense to have project-specific overrides. We could have a "different" config format that describes just those fields. Anything related to scripts/keybindings/gui seem inappropriate for project-specific overrides. Maybe the This can be done relatively sanely in the decoding logic by building up a struct that has pointers to the primary struct and unmarshalling into that. type RepoConfig struct {
Git *GitConfig `yaml:"git"`
}
...
var repoConfig = RepoConfig{
Git: &base.Git,
}
yaml.Unmarshal(content, &repoConfig) |
I worry @duckbrain that if we only allow a subset of the config to be used that:
Another option is to add a popup when lazygit encounters a new config file on startup that gets the user to confirm that they trust the file: then we can add the path to an array so that next time the user opens lazygit the config is merged with the global one. I think this would work great: it's a minor inconvenience the first time you open lazygit with that config, but it's only a one-time cost. Vscode does similar things (it's always asking me if I trust something) I think having a file in the workspace that the user can choose to either ignore or exclude should work. For the case of having one config apply to many repos, perhaps we could also look for files in parent directories, all the way to the root? Not sure how well that gels with your setup @afhoffman . |
I think the best option for the security concern is to filter out all those config options that could be abused (all that can be set to a command, basically). This is not very hard to do, technically, and the only challenge is that we need to carefully audit the set of configs that need to be filtered out.
If these are users who want to use their own local config file, then they can simply use the one in
I don't understand, can you give an example?
Personally I hate all this trust stuff that VS Code has started to ask me about a while ago, so I globally turned that off. So I'm not in favor of this option. Also, we would probably have to remember a hash of the file (or something like that), so that we can ask again whenever it has changed. I had started to work on this topic quite a while ago, but then paused it again when I ran into issues that I didn't anticipate. The biggest issue is that we need to reload config files while lazygit is running, but some configs are only read once at program start. Examples are properties that are set on views, e.g. The upside is that once we have solved that somehow, we could reload the config file not only when switching repos, but any time it changed. We'd just have to remember the modification date(s), and check them each time our terminal window receives focus. I would find this very attractive. |
What concerns me is that some of the things we would want to configure on a per-repo basis are exactly the things that pose security risks e.g. having repo-specific custom commands or configuring a repo-specific editor (that doesn't use a preset)
My argument is that we could do an audit now but later on forget to exclude some new config key that involves executing code and then we'd be back to having a security vulnerability again.
I agree that would be a dream. |
Another thing that just came to mind on the topic of excluding keys vs requiring user confirmation: vscode's incessant trust checks can be annoying but the 99% typical case is that a user creates one or two config files for themselves and has to confirm them once. Maybe a workplace has a lazygit config that's in version control. But it's going to be rare. I would much prefer the trust check if it means I have full control over what I put in my per-repo config. |
Maybe I wasn't clear enough on that, but my proposal is to support both |
I'm still uneasy about it. Putting everything on the table:
Trust popup:
Thoughts? |
Going back to Stefan's original use-cases, what about That completely avoids the problem of block lists being default-allow for newly introduced config settings. It may also avoid the need to think (yet) about trusted/untrusted repos, since I suspect there really aren't use cases for standardizing any setting that specifies a command. |
@jesseduffield made it pretty clear that he is against that, and actually I agree now. I find it totally plausible that someone has a repo containing files of a certain special kind, and provides a bunch of custom lazygit commands for working with them. So yes, I think the trust popup is the way to go. There are still some questions around that, but I think they can all be solved in some way. For example, a repo might start out without a But actually we can tell whether a file is versioned or not, so we could put up the prompt only if it is; that would solve it. (I think -- security is tricky, it's so easy to miss cases.) |
Support for unversioned config files (in |
For some of lazygit's config options it would be good to have different settings for different repos; some examples are:
git.commit.signOff
(true only in those open-source repos that require it)git.commit.wrapCommitMessageAt
(if/when we merge Auto-wrap commit message while typing #3173)git.log.order
(which you may want to set to 'top-order' for most repos, but to 'default' for the really huge ones, for performance reasons)It would also allow us to get rid of the weird
git.commitPrefixes
map and have a singlegit.commitPrefix
setting.The obvious way to support this is to allow a local config file in the git repo that overrides the global one, much like git itself does it with
~/.gitconfig
vs..git/config
.There are two possible options where we could put the file:
.git
folder (.git/lazygit.yml
)./.lazygit.yml
)Some considerations for each:
.gitignore
.vscode/settings.json
If we don't want to make the decision, we could also read files from both locations and let the user decide where to put it. But then we at least need to define the precedence.
Another alternative might be to put custom lazygit configs into git's config file; this is the approach that git gui takes. Since git uses the init format, they would have to be translated to yaml from there, which I'm not sure is always possible. I find this approach less promising for us.
The text was updated successfully, but these errors were encountered: