Junio C Hamano <gitster@xxxxxxxxx> writes:
> Karthik Nayak <karthik.188@xxxxxxxxx> writes:
>
>> The 'builtin/reflog.c' file uses the 'the_repository' global variable
>> directly and also via 'git_config()'. Since this is a builtin command
>> which has access to the 'struct repository', drop usage of the global
>> variable and use the available repository struct.
>>
>> With this, remove the 'USE_THE_REPOSITORY_VARIABLE' macro from the file.
>
> I suspect that this is not quite right.
>
> $ cd w/git.git/; make
> $ ./git-reflog list -h
> usage: git reflog list
> $ cd .. # not a repository
> $ git.git/git-reflog list -h
> fatal: not a git repository (or any of the parent directories): .git
> $ git.git/git-reflog -h
> usage: git reflog [show] ...
>
> but I also suspect that it is mostly due to the original program
> structure that uses OPT_SUBCOMMAND() that the subcommands fail to
> respond to "-h" unlike the top-level command, so this may not be a
> regression. I do think however that this change is making it harder
> to fix.
>
Hmm. But this is the existing behavior, no?
# Inside a git directory
$ eza .git
b4.template branches COMMIT_EDITMSG config description
FETCH_HEAD filter-repo HEAD hooks index info objects refs
reftable rr-cache
$ git reflog -h
usage: git reflog [show] [<log-options>] [<ref>]
or: git reflog list
or: git reflog expire [--expire=<time>] [--expire-unreachable=<time>]
[--rewrite] [--updateref] [--stale-fix]
[--dry-run | -n] [--verbose] [--all
[--single-worktree] | <refs>...]
or: git reflog delete [--rewrite] [--updateref]
[--dry-run | -n] [--verbose] <ref>@{<specifier>}...
or: git reflog exists <ref>
$ git reflog list -h
usage: git reflog list
$ ~/code/git/build/bin-wrappers/git reflog -h
usage: git reflog [show] [<log-options>] [<ref>]
or: git reflog list
or: git reflog expire [--expire=<time>] [--expire-unreachable=<time>]
[--rewrite] [--updateref] [--stale-fix]
[--dry-run | -n] [--verbose] [--all
[--single-worktree] | <refs>...]
or: git reflog delete [--rewrite] [--updateref]
[--dry-run | -n] [--verbose] <ref>@{<specifier>}...
or: git reflog exists <ref>
or: git reflog drop [--all | <refs>...]
$ ~/code/git/build/bin-wrappers/git reflog list -h
usage: git reflog list
# Outside a git repository
$ eza .git
".git": No such file or directory (os error 2)
$ git reflog -h
usage: git reflog [show] [<log-options>] [<ref>]
or: git reflog list
or: git reflog expire [--expire=<time>] [--expire-unreachable=<time>]
[--rewrite] [--updateref] [--stale-fix]
[--dry-run | -n] [--verbose] [--all
[--single-worktree] | <refs>...]
or: git reflog delete [--rewrite] [--updateref]
[--dry-run | -n] [--verbose] <ref>@{<specifier>}...
or: git reflog exists <ref>
$ git reflog list -h
fatal: not a git repository (or any of the parent directories): .git
$ ~/code/git/build/bin-wrappers/git reflog -h
usage: git reflog [show] [<log-options>] [<ref>]
or: git reflog list
or: git reflog expire [--expire=<time>] [--expire-unreachable=<time>]
[--rewrite] [--updateref] [--stale-fix]
[--dry-run | -n] [--verbose] [--all
[--single-worktree] | <refs>...]
or: git reflog delete [--rewrite] [--updateref]
[--dry-run | -n] [--verbose] <ref>@{<specifier>}...
or: git reflog exists <ref>
or: git reflog drop [--all | <refs>...]
$ ~/code/git/build/bin-wrappers/git reflog list -h
fatal: not a git repository (or any of the parent directories): .git
Seems like the behavior is the same with as with (git 2.48.1).
> In any case, when you are adding a new feature, I would strongly
> prefer you did *not* take it hostage to unrelated internal clean-up
> with a dubious value. For the library-ish parts of the system
> (e.g. reflog.c at the top-level), not depending on the_repository is
> absolutely the good thing to do, but the top level cmd_foo() are not
> meant to be called as helper functions repeatedly with arbirary
> repository instance, and a churn like this one, only to mollify the
> USE_THE_REPOSITORY_VARIABLE macro, does not deserve to take a more
> interesting (in the sense that it improves the life of end users)
> change hostage by pretending to be its prerequisite clean-up.
>
> Thanks.
But point taken, I'll drop this patch in the next version! Thanks
Attachment:
signature.asc
Description: PGP signature
