On 23/03/14 10:43AM, Junio C Hamano wrote:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
Hi Junio,
>
> > Since referring to in-tree attributes is often useful with any
> > command, not limited to "diff", I wonder if it is feasible to add
> > support for the --attr-source=<tree> option globally, instead of
> > implementing such an option piecemeal. If your "git diff" in a bare
> > repository starts honoring attributes recorded in HEAD, don't your
> > users expect your "git log" and "git show" to also honor them?
>
> Just for illustration, here is one way to do so.
>
> The implementation goes in the opposite direction from the more
> recent trend, which is why I am not making it an official patch, but
Could you explain why this goes against the "more recent trend" for my
understanding?
> with this you can do things like:
>
> $ git --attr-source=e83c5163 check-attr whitespace cache.h
> cache.h: whitespace: unspecified
> $ git --attr-source=e2f6331a142^ check-attr whitespace cache.h
> cache.h: whitespace: set
> $ git --attr-source=HEAD check-attr whitespace cache.h
> cache.h: whitespace: indent,trail,space
I like the idea of an option that is global. For git-check-attr however, we
already have a --source option. Not sure how big of a deal that is. If we wanted
to, we could put --source on a deprecation path by making it hidden for a
while, etc.
>
> where e83c5163 is the very first version of Git from 2005 where
> .gitattributes did not exist, e2f6331a142^ is the last version
> before we set whitespace to indent,trail,space, and HEAD is a more
> recent version of our source tree.
>
> In the following illustration patch, the attr-source tree object
> name is kept as a string as long as possible and at the very last
> minute when we call git_check_attr() for the first time we turn it
> into an object id. This is because at the very early stage when we
> parse the global option we may not even know where our repository is
> (hence do not have enough information to parse HEAD). We also figure
> out is_bare_repository() late in the process for the same reason.
>
>
>
> attr.c | 29 +++++++++++++++++++++++++++++
> attr.h | 6 ++++++
> git.c | 3 +++
> 3 files changed, 38 insertions(+)
>
> diff --git c/attr.c w/attr.c
> index 1053dfcd4b..2fd6e0eab2 100644
> --- c/attr.c
> +++ w/attr.c
> @@ -1165,12 +1165,41 @@ static void collect_some_attrs(struct index_state *istate,
> fill(path, pathlen, basename_offset, check->stack, check->all_attrs, rem);
> }
>
> +static const char *default_attr_source_tree_object_name;
> +
> +void set_git_attr_source(const char *tree_object_name)
> +{
> + default_attr_source_tree_object_name = xstrdup(tree_object_name);
> +}
> +
> +
> +static struct object_id *default_attr_source(void)
> +{
> + static struct object_id attr_source;
> +
> + if (is_null_oid(&attr_source)) {
> + if (!default_attr_source_tree_object_name &&
> + is_bare_repository())
> + default_attr_source_tree_object_name = "HEAD";
> +
> + if (default_attr_source_tree_object_name &&
> + is_null_oid(&attr_source))
> + get_oid(default_attr_source_tree_object_name, &attr_source);
> + }
> + if (is_null_oid(&attr_source))
> + return NULL;
> + return &attr_source;
> +}
> +
> void git_check_attr(struct index_state *istate,
> const struct object_id *tree_oid, const char *path,
> struct attr_check *check)
> {
> int i;
>
> + if (!tree_oid)
> + tree_oid = default_attr_source();
> +
> collect_some_attrs(istate, tree_oid, path, check);
>
> for (i = 0; i < check->nr; i++) {
> diff --git c/attr.h w/attr.h
> index 9884ea2bc6..a77e3713b2 100644
> --- c/attr.h
> +++ w/attr.h
> @@ -135,6 +135,12 @@ struct git_attr;
> struct all_attrs_item;
> struct attr_stack;
>
> +/*
> + * The textual object name for the tree-ish used by git_check_attr()
> + * when NULL is given to tree_oid.
> + */
> +void set_git_attr_source(const char *);
> +
> /*
> * Given a string, return the gitattribute object that
> * corresponds to it.
> diff --git c/git.c w/git.c
> index 6171fd6769..21bddc5718 100644
> --- c/git.c
> +++ w/git.c
> @@ -4,6 +4,7 @@
> #include "help.h"
> #include "run-command.h"
> #include "alias.h"
> +#include "attr.h"
> #include "shallow.h"
>
> #define RUN_SETUP (1<<0)
> @@ -307,6 +308,8 @@ static int handle_options(const char ***argv, int *argc, int *envchanged)
> } else {
> exit(list_cmds(cmd));
> }
> + } else if (skip_prefix(cmd, "--attr-source=", &cmd)) {
> + set_git_attr_source(cmd);
> } else {
> fprintf(stderr, _("unknown option: %s\n"), cmd);
> usage(git_usage_string);
