Skip to content

Add core.editor key#1178

Merged
Byron merged 3 commits intoGitoxideLabs:mainfrom
lewiscowper:patch-1
Dec 12, 2023
Merged

Add core.editor key#1178
Byron merged 3 commits intoGitoxideLabs:mainfrom
lewiscowper:patch-1

Conversation

@lewiscowper
Copy link
Contributor

@lewiscowper lewiscowper commented Dec 12, 2023

Happy to adjust based on feedback, I wanted to add a feature to danobi/prr#36 where we would use $EDITOR as a fallback, and use core.editor as the primary source, to match expectations with git itself, but the field isn't (yet) supported here, so this PR adds the core.editor field, I hope.

There is a test failing, but it seems like an unrelated failure, and it might be my local setup interfering, as it's about file::init::comfort::from_git_dir, so I want to open the PR to let the CI take a shot at it.

Looking forward to your feedback, feel free to edit and adjust as you see fit.

lewiscowper and others added 2 commits December 12, 2023 14:03
@lewiscowper
Add core.editor key
ff71e07
@Byron
Differentiate between Executable and Program
56d1d09 
It's somewhat confusing, but it seems sometimes `git` uses a shell, but other
times it doesn't. And that is differentiate by the type system.

It would still have to be (re)validated at some point if this is actually the case,
but I guess there was a reason for this in the first place.
@Byron
Copy link
Member

Byron commented Dec 12, 2023

Thanks a lot for contributing!

I think this is exactly how it would be done, and seeing Executable used reminded me of a couple of fixes I had to apply to the existing code as I apparently got that wrong in some places :D. It also turns out that Git uses the shell for core.editor (see here), so it would be identified as Program there - I made the fix already and will merge when CI is happy.

Please note that when using the value of core.editor, you'd want to assure the value comes from a trusted configuration file (an example could be trusted_path()) - unfortunately there is no such method yet for programs, but in the end it's a call to string_filter() with the filter being… it looks like the default filter isn't publicly available.

Let me fix that…

@Byron
feat: add config::Snapshot::trusted_program(). (GitoxideLabs#1178)
3f84213 
That way it's possible to obtain an executable, program or script
from a key in the configuration that is in a trusted section of the
configuration.

This goes along with a new `command` feature that brings in the `command`
module at the top level to be able to execute such commands.
@Byron
Copy link
Member

Byron commented Dec 12, 2023

Now there also is a trusted_program() method that can be used to obtain core.editor, and you will also find command in the top-level of the crate that helps with executing these commands 'git-style'.

I hope that will help.

@Byron Byron merged commit 20dce42 into GitoxideLabs:main Dec 12, 2023
@lewiscowper lewiscowper deleted the patch-1 branch December 12, 2023 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lewiscowper @Byron