[PATCH v2 06/10] rtc: add rtc command

From: Rasmus Villemoes <rasmus.villemoes@prevas.dk>
To: u-boot@lists.denx.de
Subject: [PATCH v2 06/10] rtc: add rtc command
Date: Tue, 2 Jun 2020 16:36:18 +0200	[thread overview]
Message-ID: <00233225-6d66-105d-9b89-afbf8224960f@prevas.dk> (raw)
In-Reply-To: <CAPnjgZ2Q5ruAY_8-efibaJjRokQaYujb6Dw5tXNtJBsTw6x0tw@mail.gmail.com>

On 02/06/2020 15.22, Simon Glass wrote:
> Hi Rasmus,
> 
> On Tue, 2 Jun 2020 at 03:13, Rasmus Villemoes
> <rasmus.villemoes@prevas.dk> wrote:
>>
>> On 31/05/2020 16.07, Simon Glass wrote:
>>> Hi Rasmus,
>>>
>>> On Tue, 19 May 2020 at 16:01, Rasmus Villemoes
>>> <rasmus.villemoes@prevas.dk> wrote:
>>>>
> 
> [..]
> 
>>>> +int do_rtc(cmd_tbl_t *cmdtp, int flag, int argc, char * const argv[])
>>>> +{
>>>> +       static int curr_rtc = 0;
>>>> +       struct udevice *dev;
>>>> +       int ret, idx;
>>>> +
>>>> +       if (argc < 2)
>>>> +               return CMD_RET_USAGE;
>>>> +
>>>> +       argc--;
>>>> +       argv++;
>>>> +
>>>> +       if (!strcmp(argv[0], "list")) {
>>>
>>> It is comment in U-Boot to just check the letters that are needed. So
>>> here you could do (*argv[0] == 'l')
>>
>> Yes, and I consider that an anti-pattern. It makes it impossible to
>> later introduce another (sub)command which starts with a
>> previously-unique prefix. Now, if that "just type a unique prefix"
>> wasn't official, so scripts were always supposed to use the full names,
>> it wouldn't be that big a problem (scripts written for later versions of
>> U-Boot, or U-Boots configured with more (sub)commands, could still fail
>> silently if used on an earlier U-Boot or one with fewer (sub)commands
>> instead of producing a "usage" error message), but
>> https://www.denx.de/wiki/view/DULG/UBootCommandLineInterface explicitly
>> mentions that as a feature (and says h can be used for help, which it
>> can't when the hash command is built in, perfectly exemplifying what I'm
>> talking about).
> 
> Hah funny. Using an abbreviation is only possible if no other command
> starts with the same leters.
> 
> It is certainly very risky to use abbreviations in scripts. I would
> not recommend it. Abbreviations are for interactive use. If you have
> auto-completion on you can use tab.

Exactly, so the ability to use the abbreviated form doesn't really buy
anything - it's risky in scripts, and interactively, it merely saves a
tab keystroke (and that's all lost in the cognitive overhead of having
to remember just what abbrev is enough).

> But here we are talking about a sub-command, which is a bit more
> controlled, in that it doesn't depend on what other commands the user
> enables.

True, but the same point applies; if I allowed "rtc w", one couldn't
easily later add an "rtc wobble" subcommand (ok, my imagination is
lacking, but you get the idea).

> Anyway, it's up to you what you want to do here.

In that case I'll keep checking for the full name of subcommands.

Thanks,
Rasmus