git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH/RFC] WIP: log: allow "-" as a short-hand for "previous branch"

From
Pranit Bauva <pranit.bauva@gmail.com>
Date
Feb 5, 2017, 14:55 UTC
Message-ID
<CAFZEwPOFPPyui=9mnccbOc-79q0URYhdGHSkcd0YyR6qe-c_zQ@mail.gmail.com>
In-Reply-To
<1486299439-2859-1-git-send-email-kannan.siddharth12@gmail.com>
Hey Siddharth,

On Sun, Feb 5, 2017 at 6:27 PM, Siddharth Kannan <kannan.siddharth12@gmail.com> wrote:

Show 113 quoted lines
> Search and replace "-" (written in the context of a branch name) in the argument
> list with "@{-1}". As per the help text of git rev-list, this includes the following four
> cases:
>
>   a. "-"
>   b. "^-"
>   c. "-..other-branch-name" or "other-branch-name..-"
>   d. "-...other-branch-name" or "other-branch-name...-"
>
> (a) and (b) have been implemented as in the previous implementations of
> this abbreviation. Namely, 696acf45 (checkout: implement "-" abbreviation, add
> docs and tests, 2009-01-17), 182d7dc4 (cherry-pick: allow "-" as
> abbreviation of '@{-1}', 2013-09-05) and 4e8115ff (merge: allow "-" as a
> short-hand for "previous branch", 2011-04-07)
>
> (c) and (d) have been implemented by using the strbuf API, growing it to the
> right size and placing "@{-1}" instead of "-"
>
> Signed-off-by: Siddharth Kannan <kannan.siddharth12@gmail.com>
> ---
> This is a patch for one of the microprojects of SoC 2017. [1]
>
> I have implemented this using multiple methods, that I have re-written again and
> again for better versions ([2]). The present version I feel is the closest that
> I could get to the existing code in the repository. This patch only uses
> functions that are commonly used in the rest of the codebase.
>
> I still have to write tests, as well as update documentation as done in 696acf45
> (checkout: implement "-" abbreviation, add docs and tests, 2009-01-17).
>
> I request your comments on this patch. Also, I have the following questions
> regarding this patch:
>
> 1. Is the approach that I have used to solve this problem fine?
> 2. Is the code I am writing in the right function? (I have put it right
> before the revisions data structure is setup, thus these changes affect only
> git-log)
>
> [1]: https://git.github.io/SoC-2017-Microprojects/
> [2]: https://github.com/git/git/compare/6e3a7b3...icyflame:7e286c9.patch (Uses
> strbufs for the starting 4 characters, and last 4 characters and compares those
> to the appropriate strings for case (c) and case (d). I edited this patch to use
> strstr instead, which avoids all the strbuf declarations)
>
>  builtin/log.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 46 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index 55d20cc..a5aac99 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -132,7 +132,7 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
>                          struct rev_info *rev, struct setup_revision_opt *opt)
>  {
>         struct userformat_want w;
> -       int quiet = 0, source = 0, mailmap = 0;
> +       int quiet = 0, source = 0, mailmap = 0, i = 0;
>         static struct line_opt_callback_data line_cb = {NULL, NULL, STRING_LIST_INIT_DUP};
>
>         const struct option builtin_log_options[] = {
> @@ -158,6 +158,51 @@ static void cmd_log_init_finish(int argc, const char **argv, const char *prefix,
>
>         if (quiet)
>                 rev->diffopt.output_format |= DIFF_FORMAT_NO_OUTPUT;
> +
> +       /*
> +        * Check if any argument has a "-" in it, which has been referred to as a
> +        * shorthand for @{-1}.  Handles methods that might be used to list commits
> +        * as mentioned in git rev-list --help
> +        */
> +
> +       for(i = 0; i < argc; ++i) {
> +               if (!strcmp(argv[i], "-")) {
> +                       argv[i] = "@{-1}";
> +               } else if (!strcmp(argv[i], "^-")) {
> +                       argv[i] = "^@{-1}";
> +               } else if (strlen(argv[i]) >= 4) {
> +
> +                       if (strstr(argv[i], "-...") == argv[i] || strstr(argv[i], "-..") == argv[i]) {
> +                               struct strbuf changed_argument = STRBUF_INIT;
> +
> +                               strbuf_addstr(&changed_argument, "@{-1}");
> +                               strbuf_addstr(&changed_argument, argv[i] + 1);
> +
> +                               strbuf_setlen(&changed_argument, strlen(argv[i]) + 4);
> +
> +                               argv[i] = strbuf_detach(&changed_argument, NULL);
> +                       }
> +
> +                       /*
> +                        * Find the first occurence, and add the size to it and proceed if
> +                        * the resulting value is NULL
> +                        */
> +                       if (!(strstr(argv[i], "...-") + 4)  ||
> +                                       !(strstr(argv[i], "..-") + 3)) {
> +                               struct strbuf changed_argument = STRBUF_INIT;
> +
> +                               strbuf_addstr(&changed_argument, argv[i]);
> +
> +                               strbuf_grow(&changed_argument, strlen(argv[i]) + 4);
> +                               strbuf_setlen(&changed_argument, strlen(argv[i]) + 4);
> +
> +                               strbuf_splice(&changed_argument, strlen(argv[i]) - 1, 5, "@{-1}", 5);
> +
> +                               argv[i] = strbuf_detach(&changed_argument, NULL);
> +                       }
> +               }
> +       }
> +
>         argc = setup_revisions(argc, argv, rev, opt);
>
>         /* Any arguments at this point are not recognized */
> --

It is highly recommended to follow the pre existing style of code and commits. In the micro project list, I think it is mentioned that this similar thing is implemented in git-merge so you should try and dig the commit history of that file to find the similar change.

If you do this, then you will find out that there is a very short and sweet way to do it. I won't directly point out the commit.

strbuf API should be used when you need to modify the contents of the string. I think you have a little confusion.

If you declare the string as,
const char *str = "foo";
then, you can also do,
str = "bar";
But you can't do,
str[1] = 'z';
I hope you get what I am saying, if not, search for it.

Regards, Pranit Bauva

Previous: Siddharth KannanNext: Junio C Hamano
Message 2 of 11 in “WIP: log: allow "-" as a short-hand for "previous branch"”
  1. WIP: log: allow "-" as a short-hand for "previous branch"Siddharth Kannan, Feb 5, 2017
  2. Pranit BauvaFeb 5, 2017
  3. Junio C HamanoFeb 6, 2017
  4. Siddharth KannanFeb 6, 2017
  5. Siddharth KannanFeb 6, 2017
  6. Junio C HamanoFeb 6, 2017
  7. Siddharth KannanFeb 7, 2017
  8. Matthieu MoyFeb 8, 2017
  9. Siddharth KannanFeb 8, 2017
  10. Matthieu MoyFeb 9, 2017
  11. Siddharth KannanFeb 9, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.