Will Palmer wrote:
> here we modify the find_commit_format function to make it recursively
> dereference aliases when they are specified. At this point, there are
> no aliases specified and there is no way to specify an alias, but the
> support is there for any which are added.
Style. Maybe:
Subject: pretty: add infrastructure for commit format aliases
Allow named commit formats to alias one another;
find_commit_format() will recursively dereference aliases when
they are specified. At this point, there are no aliases
specified and there is no way to specify an alias, but the
support is there for any which are added.
If an alias loop is detected, the function die()s.
[...]
> -static struct cmt_fmt_map *find_commit_format(const char *sought)
> +static struct cmt_fmt_map *find_commit_format_recursive(const char *sought,
> + const char *original,
> + int num_redirections)
> {
> struct cmt_fmt_map *found = NULL;
> size_t found_match_len;
> int i;
>
> - if (!commit_formats)
> - setup_commit_formats();
> + if (num_redirections >= commit_formats_len) {
> + die("invalid --pretty format: '%s' references an alias which "
> + "points to itself", original);
> + return NULL;
nitpicks:
1. If the caller might like the chance to add more information or
recover (as in code used by git daemon or that might become part of
libgit2), you can error() and return NULL. This would print an
"error: " message and let the caller take care of exiting.
Otherwise, this should probably die() (which prints a "fatal: "
message) without returning anything.
Not important, of course; just something to avoid confusion for
the reader and static analyzers.
2. It might be helpful to wrap differently in case someone tries to
grep for "alias which points to itself" after encountering the
error message.
With or without the changes mentioned above:
Reviewed-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
Nice, thanks. I hope GCC notices the tail recursion (though since
this is not so performance critical for git afaict, if it doesn’t, it
is probably better to fix that in GCC than make the git code uglier).
diff --git a/pretty.c b/pretty.c
index 02665d0..0ba056a 100644
--- a/pretty.c
+++ b/pretty.c
@@ -56,11 +56,10 @@ static struct cmt_fmt_map *find_commit_format_recursive(const char *sought,
size_t found_match_len;
int i;
- if (num_redirections >= commit_formats_len) {
- die("invalid --pretty format: '%s' references an alias which "
- "points to itself", original);
- return NULL;
- }
+ if (num_redirections >= commit_formats_len)
+ die("invalid --pretty format: "
+ "'%s' references an alias which points to itself",
+ original);
for (i = 0; i < commit_formats_len; i++) {
size_t match_len;
--
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html