Hello, On Fri, Nov 11, 2016 at 10:59 AM, Jacob Keller <jacob.kel...@gmail.com> wrote: > On Thu, Nov 10, 2016 at 9:36 AM, Karthik Nayak <karthik....@gmail.com> wrote: >> On Wed, Nov 9, 2016 at 4:57 AM, Jacob Keller <jacob.kel...@gmail.com> wrote: >> >> That does make sense, It would also not error out when we use >> %(objectname:short=) and >> not specify the length. Idk, if that's desirable or not. But it does >> make the code a little more >> confusing to read at the same time. >> > > I am not sure that would be the case. If you see "objectname:short" > you trreat this as if they had passed "objectname:short=<default > abbrev>" but if you see "objectname:short=" you die, no? >
Sorry, my bad. On Fri, Nov 11, 2016 at 5:02 AM, Junio C Hamano <gits...@pobox.com> wrote: > Karthik Nayak <karthik....@gmail.com> writes: > >> else if (!strcmp(arg, "short")) >> - atom->u.objectname = O_SHORT; >> - else >> + atom->u.objectname.option = O_SHORT; >> + else if (skip_prefix(arg, "short=", &arg)) { >> + atom->u.objectname.option = O_LENGTH; >> + if (strtoul_ui(arg, 10, &atom->u.objectname.length) || >> + atom->u.objectname.length == 0) >> + die(_("positive value expected objectname:short=%s"), >> arg); >> + if (atom->u.objectname.length < MINIMUM_ABBREV) >> + atom->u.objectname.length = MINIMUM_ABBREV; >> + } else >> die(_("unrecognized %%(objectname) argument: %s"), arg); >> } > > Users who want to use the default-abbrev, i.e. the autoscaling one > introduced recently, must use "short", not "short=-1", with this > code (especially with the "must be at least MINIMUM_ABBREV" logic), > but I do not think it is a problem, so I think this is good. > I think I'll leave this as it is. If that's okay -- Regards, Karthik Nayak