> +--attr-source=<tree-ish>:: > -+ Read gitattributes from <tree-ish> instead of the worktree. > ++ Read gitattributes from <tree-ish> instead of the worktree. See > ++ linkgit:gitrevisions[7]. I think it's more sensible to link to gitattributes(5) instead of gitrevisions(7) > +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); > +} > + > + One empty line is enough here. > +static void compute_default_attr_source(struct object_id *attr_source) > +{ > + int from_env = 0; > + > + if (!default_attr_source_tree_object_name) { > + default_attr_source_tree_object_name = getenv(GIT_ATTR_SOURCE); > + from_env = 1; > + } > + > + if (!default_attr_source_tree_object_name || !is_null_oid(attr_source)) > + return; > + > + if (repo_get_oid_treeish(the_repository, default_attr_source_tree_object_name, attr_source)) { > + if (from_env) > + die(_("bad --attr-source object")); > + else > + die(_("bad GIT_ATTR_SOURCE")); I think it would be better to have just the following instead of the 4 lines above: die(_("invalid tree object from --attr-source flag or GIT_ATTR_SOURCE env variable")); as a bad GIT_ATTR_SOURCE in a subprocess could come from a bad --attr-source in the main process. And this way the from_env variable is not needed. > + } > +} The rest looks good to me, thanks!