About more loose parameter sequence requirement

About more loose parameter sequence requirement

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 773 bytes --]

Hi David,

I understand that btrfs-progs introduced restrict parameter/option order
to distinguish global and sub-command parameter/option.

However it's really annoying if one just want to append some new options
to previous command:

E.g.
# btrfs check /dev/data/btrfs
# !! --check-data-csum

The last command will fail as current btrfs-progs doesn't allow any
option after parameter.


Despite the requirement to distinguish global and subcommand
option/parameter, is there any other requirement for such restrict
option-first-parameter-last policy?

If I could implement a enhanced getopt to allow more loose order inside
subcomand while still can distinguish global option, will it be accepted
(if it's quality is acceptable) ?

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: About more loose parameter sequence requirement

[David Sterba]

On Thu, Jun 14, 2018 at 03:17:45PM +0800, Qu Wenruo wrote:
> I understand that btrfs-progs introduced restrict parameter/option order
> to distinguish global and sub-command parameter/option.
> 
> However it's really annoying if one just want to append some new options
> to previous command:
> 
> E.g.
> # btrfs check /dev/data/btrfs
> # !! --check-data-csum
> 
> The last command will fail as current btrfs-progs doesn't allow any
> option after parameter.
> 
> 
> Despite the requirement to distinguish global and subcommand
> option/parameter, is there any other requirement for such restrict
> option-first-parameter-last policy?

I'd say that it's a common and recommended pattern. Getopt is able to
reorder the parameters so mixed options and non-options are accepted,
unless POSIXLY_CORRECT (see man getopt(3)) is not set. With the more
strict requirement, 'btrfs' option parser works the same regardless of
that.

> If I could implement a enhanced getopt to allow more loose order inside
> subcomand while still can distinguish global option, will it be accepted
> (if it's quality is acceptable) ?

I think it's not worth updating the parser just to support an IMHO
narrow usecase.

Re: About more loose parameter sequence requirement

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1582 bytes --]



On 2018年06月18日 19:34, David Sterba wrote:
> On Thu, Jun 14, 2018 at 03:17:45PM +0800, Qu Wenruo wrote:
>> I understand that btrfs-progs introduced restrict parameter/option order
>> to distinguish global and sub-command parameter/option.
>>
>> However it's really annoying if one just want to append some new options
>> to previous command:
>>
>> E.g.
>> # btrfs check /dev/data/btrfs
>> # !! --check-data-csum
>>
>> The last command will fail as current btrfs-progs doesn't allow any
>> option after parameter.
>>
>>
>> Despite the requirement to distinguish global and subcommand
>> option/parameter, is there any other requirement for such restrict
>> option-first-parameter-last policy?
> 
> I'd say that it's a common and recommended pattern. Getopt is able to
> reorder the parameters so mixed options and non-options are accepted,
> unless POSIXLY_CORRECT (see man getopt(3)) is not set. With the more
> strict requirement, 'btrfs' option parser works the same regardless of
> that.

But currently it doesn't work.
Just as the example, btrfs check /dev/data/btrfs --check-data-sum won't
work.
It's different from a lot of other common programs.

> 
>> If I could implement a enhanced getopt to allow more loose order inside
>> subcomand while still can distinguish global option, will it be accepted
>> (if it's quality is acceptable) ?
> 
> I think it's not worth updating the parser just to support an IMHO
> narrow usecase.

I'll try to use the getopt() and keeps the current structure if possible.

Thanks,
Qu

> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: About more loose parameter sequence requirement

[Hugo Mills]

[-- Attachment #1: Type: text/plain, Size: 1873 bytes --]

On Mon, Jun 18, 2018 at 01:34:32PM +0200, David Sterba wrote:
> On Thu, Jun 14, 2018 at 03:17:45PM +0800, Qu Wenruo wrote:
> > I understand that btrfs-progs introduced restrict parameter/option order
> > to distinguish global and sub-command parameter/option.
> > 
> > However it's really annoying if one just want to append some new options
> > to previous command:
> > 
> > E.g.
> > # btrfs check /dev/data/btrfs
> > # !! --check-data-csum
> > 
> > The last command will fail as current btrfs-progs doesn't allow any
> > option after parameter.
> > 
> > 
> > Despite the requirement to distinguish global and subcommand
> > option/parameter, is there any other requirement for such restrict
> > option-first-parameter-last policy?
> 
> I'd say that it's a common and recommended pattern. Getopt is able to
> reorder the parameters so mixed options and non-options are accepted,
> unless POSIXLY_CORRECT (see man getopt(3)) is not set. With the more
> strict requirement, 'btrfs' option parser works the same regardless of
> that.

   I got bitten by this the other day. I put an option flag at the end
of the line, after the mountpoint, and it refused to work.

   I would definitely prefer it if it parsed options in any
position. (Or at least, any position after the group/command
parameters).

   Hugo.

> > If I could implement a enhanced getopt to allow more loose order inside
> > subcomand while still can distinguish global option, will it be accepted
> > (if it's quality is acceptable) ?
> 
> I think it's not worth updating the parser just to support an IMHO
> narrow usecase.

-- 
Hugo Mills             | Turning, pages turning in the widening bath,
hugo@... carfax.org.uk | The spine cannot bear the humidity.
http://carfax.org.uk/  | Books fall apart; the binding cannot hold.
PGP: E2AB1DE4          | Page 129 is loosed upon the world.               Zarf

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: About more loose parameter sequence requirement

[David Sterba]

On Mon, Jun 18, 2018 at 07:40:44PM +0800, Qu Wenruo wrote:
> 
> 
> On 2018年06月18日 19:34, David Sterba wrote:
> > On Thu, Jun 14, 2018 at 03:17:45PM +0800, Qu Wenruo wrote:
> >> I understand that btrfs-progs introduced restrict parameter/option order
> >> to distinguish global and sub-command parameter/option.
> >>
> >> However it's really annoying if one just want to append some new options
> >> to previous command:
> >>
> >> E.g.
> >> # btrfs check /dev/data/btrfs
> >> # !! --check-data-csum
> >>
> >> The last command will fail as current btrfs-progs doesn't allow any
> >> option after parameter.
> >>
> >>
> >> Despite the requirement to distinguish global and subcommand
> >> option/parameter, is there any other requirement for such restrict
> >> option-first-parameter-last policy?
> > 
> > I'd say that it's a common and recommended pattern. Getopt is able to
> > reorder the parameters so mixed options and non-options are accepted,
> > unless POSIXLY_CORRECT (see man getopt(3)) is not set. With the more
> > strict requirement, 'btrfs' option parser works the same regardless of
> > that.
> 
> But currently it doesn't work.
> Just as the example, btrfs check /dev/data/btrfs --check-data-sum won't
> work.
> It's different from a lot of other common programs.

With POSIXLY_CORRECT set, it expectedly won't work. With POSIXLY_CORRECT
unset, it would work in general, but not for 'btrfs'.

As this is per-application decision I find it ok, besides that I also
find the 'options anywhere' pattern bad. Does it really save you that
much time while typing the commands with new arguments? There are
movement shortcuts to jump by words, you get the previous command by
up-arrow. About the same number of keystrokes.

New code needs to be tested, documented and maintained, that's the cost
I find too high for something that's convenience for people who are used
to some shell builtins.

RE: About more loose parameter sequence requirement

[Paul Jones]

> -----Original Message-----
> From: linux-btrfs-owner@vger.kernel.org <linux-btrfs-
> owner@vger.kernel.org> On Behalf Of Hugo Mills
> Sent: Monday, 18 June 2018 9:44 PM
> To: dsterba@suse.cz; Qu Wenruo <quwenruo.btrfs@gmx.com>; linux-
> btrfs@vger.kernel.org
> Subject: Re: About more loose parameter sequence requirement
> 
> On Mon, Jun 18, 2018 at 01:34:32PM +0200, David Sterba wrote:
> > On Thu, Jun 14, 2018 at 03:17:45PM +0800, Qu Wenruo wrote:
> > > I understand that btrfs-progs introduced restrict parameter/option
> > > order to distinguish global and sub-command parameter/option.
> > >
> > > However it's really annoying if one just want to append some new
> > > options to previous command:
> > >
> > > E.g.
> > > # btrfs check /dev/data/btrfs
> > > # !! --check-data-csum
> > >
> > > The last command will fail as current btrfs-progs doesn't allow any
> > > option after parameter.
> > >
> > >
> > > Despite the requirement to distinguish global and subcommand
> > > option/parameter, is there any other requirement for such restrict
> > > option-first-parameter-last policy?
> >
> > I'd say that it's a common and recommended pattern. Getopt is able to
> > reorder the parameters so mixed options and non-options are accepted,
> > unless POSIXLY_CORRECT (see man getopt(3)) is not set. With the more
> > strict requirement, 'btrfs' option parser works the same regardless of
> > that.
> 
>    I got bitten by this the other day. I put an option flag at the end of the line,
> after the mountpoint, and it refused to work.
> 
>    I would definitely prefer it if it parsed options in any position. (Or at least,
> any position after the group/command parameters).


Same with me - I do it all the time. I type the arguments as I think of them, which is usually back-to-front of what is required.
Eg. Btrfs check this mountpoint, oh yeah, and use this specific option = fail.
Arrow arrow arrow arrow arrow....


Paul.

Re: About more loose parameter sequence requirement

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 2780 bytes --]



On 2018年06月18日 20:02, David Sterba wrote:
> On Mon, Jun 18, 2018 at 07:40:44PM +0800, Qu Wenruo wrote:
>>
>>
>> On 2018年06月18日 19:34, David Sterba wrote:
>>> On Thu, Jun 14, 2018 at 03:17:45PM +0800, Qu Wenruo wrote:
>>>> I understand that btrfs-progs introduced restrict parameter/option order
>>>> to distinguish global and sub-command parameter/option.
>>>>
>>>> However it's really annoying if one just want to append some new options
>>>> to previous command:
>>>>
>>>> E.g.
>>>> # btrfs check /dev/data/btrfs
>>>> # !! --check-data-csum
>>>>
>>>> The last command will fail as current btrfs-progs doesn't allow any
>>>> option after parameter.
>>>>
>>>>
>>>> Despite the requirement to distinguish global and subcommand
>>>> option/parameter, is there any other requirement for such restrict
>>>> option-first-parameter-last policy?
>>>
>>> I'd say that it's a common and recommended pattern. Getopt is able to
>>> reorder the parameters so mixed options and non-options are accepted,
>>> unless POSIXLY_CORRECT (see man getopt(3)) is not set. With the more
>>> strict requirement, 'btrfs' option parser works the same regardless of
>>> that.
>>
>> But currently it doesn't work.
>> Just as the example, btrfs check /dev/data/btrfs --check-data-sum won't
>> work.
>> It's different from a lot of other common programs.
> 
> With POSIXLY_CORRECT set, it expectedly won't work. With POSIXLY_CORRECT
> unset, it would work in general, but not for 'btrfs'.
> 
> As this is per-application decision I find it ok, besides that I also
> find the 'options anywhere' pattern bad. Does it really save you that
> much time while typing the commands with new arguments?

Personally speaking, yes.

> There are
> movement shortcuts to jump by words, you get the previous command by
> up-arrow. About the same number of keystrokes.

Not really, normally just "!! <new options>", where I don't even need to
move my fingers to arrow keys.
(I'm all ears about extra bash shortcuts on this)

> 
> New code needs to be tested, documented and maintained, that's the cost
> I find too high for something that's convenience for people who are used
> to some shell builtins.

The biggest problem is, the behavior isn't even consistent across
btrfs-progs.
mkfs.btrfs accept such out-of-order parameters while btrfs not.

And most common tools, like commands provided by coretutil, they don't
care about the order.
The only daily exception is 'scp', which I found it pretty unhandy.

And just as Paul and Hugo, I think there are quite some users preferring
out-of-order parameter/options.


I also understand the maintenance burden, but at least let's try if
there are better solution for this.

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: About more loose parameter sequence requirement

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 5352 bytes --]



On 2018年06月18日 20:02, David Sterba wrote:
> On Mon, Jun 18, 2018 at 07:40:44PM +0800, Qu Wenruo wrote:
>>
>>
>> On 2018年06月18日 19:34, David Sterba wrote:
>>> On Thu, Jun 14, 2018 at 03:17:45PM +0800, Qu Wenruo wrote:
>>>> I understand that btrfs-progs introduced restrict parameter/option order
>>>> to distinguish global and sub-command parameter/option.
>>>>
>>>> However it's really annoying if one just want to append some new options
>>>> to previous command:
>>>>
>>>> E.g.
>>>> # btrfs check /dev/data/btrfs
>>>> # !! --check-data-csum
>>>>
>>>> The last command will fail as current btrfs-progs doesn't allow any
>>>> option after parameter.
>>>>
>>>>
>>>> Despite the requirement to distinguish global and subcommand
>>>> option/parameter, is there any other requirement for such restrict
>>>> option-first-parameter-last policy?
>>>
>>> I'd say that it's a common and recommended pattern. Getopt is able to
>>> reorder the parameters so mixed options and non-options are accepted,
>>> unless POSIXLY_CORRECT (see man getopt(3)) is not set. With the more
>>> strict requirement, 'btrfs' option parser works the same regardless of
>>> that.
>>
>> But currently it doesn't work.
>> Just as the example, btrfs check /dev/data/btrfs --check-data-sum won't
>> work.
>> It's different from a lot of other common programs.
> 
> With POSIXLY_CORRECT set, it expectedly won't work. With POSIXLY_CORRECT
> unset, it would work in general, but not for 'btrfs'.
> 
> As this is per-application decision I find it ok, besides that I also
> find the 'options anywhere' pattern bad. Does it really save you that
> much time while typing the commands with new arguments? There are
> movement shortcuts to jump by words, you get the previous command by
> up-arrow. About the same number of keystrokes.
> 
> New code needs to be tested, documented and maintained, that's the cost
> I find too high for something that's convenience for people who are used
> to some shell builtins.
> 

Well, after some testing, the result looks pretty strange on my side.

With the testing code (*), mostly just copied from check/main.c, it
works to detect the final --check-data-csum without problem.

I'm originally planning to use '-' to make getopt_long() to keep the
original order, but the experiment I did shows it unnecessary.

And furthermore, even changing the optstring of btrfs check
getopt_long() with leading '-', it still doesn't work as expected to
detect non-option parameter.

Is there anything wrong/special in btrfs-progs getopt_long() usage?

Thanks,
Qu

*:
---
#include <stdio.h>
#include <getopt.h>
#include <unistd.h>

int main(int argc, char** argv)
{
	char *real_argv[] = { "btrfs check", "device", "--check-data-csum" , NULL};
	int real_argc = sizeof(real_argv) / sizeof(char *) - 1;

	while (1) {
		enum { GETOPT_VAL_REPAIR = 257, GETOPT_VAL_INIT_CSUM,
			GETOPT_VAL_INIT_EXTENT, GETOPT_VAL_CHECK_CSUM,
			GETOPT_VAL_READONLY, GETOPT_VAL_CHUNK_TREE,
			GETOPT_VAL_MODE, GETOPT_VAL_CLEAR_SPACE_CACHE,
			GETOPT_VAL_FORCE };
		static const struct option long_options[] = {
			{ "super", required_argument, NULL, 's' },
			{ "repair", no_argument, NULL, GETOPT_VAL_REPAIR },
			{ "readonly", no_argument, NULL, GETOPT_VAL_READONLY },
			{ "init-csum-tree", no_argument, NULL,
				GETOPT_VAL_INIT_CSUM },
			{ "init-extent-tree", no_argument, NULL,
				GETOPT_VAL_INIT_EXTENT },
			{ "check-data-csum", no_argument, NULL,
				GETOPT_VAL_CHECK_CSUM },
			{ "backup", no_argument, NULL, 'b' },
			{ "subvol-extents", required_argument, NULL, 'E' },
			{ "qgroup-report", no_argument, NULL, 'Q' },
			{ "tree-root", required_argument, NULL, 'r' },
			{ "chunk-root", required_argument, NULL,
				GETOPT_VAL_CHUNK_TREE },
			{ "progress", no_argument, NULL, 'p' },
			{ "mode", required_argument, NULL,
				GETOPT_VAL_MODE },
			{ "clear-space-cache", required_argument, NULL,
				GETOPT_VAL_CLEAR_SPACE_CACHE},
			{ "force", no_argument, NULL, GETOPT_VAL_FORCE },
			{ NULL, 0, NULL, 0}
		};
		int c;

		c = getopt_long(real_argc, real_argv, "as:br:pEQ", long_options, NULL);
		if (c < 0)
			break;
		switch (c) {
			case 'a':
			case 'b':
			case 'Q':
			case 'E':
			case 'p':
				printf("option: %c\n", c);
				break;
			case 's':
			case 'r':
				printf("option: %c %s\n", c, optarg);
				break;
			case GETOPT_VAL_CHUNK_TREE:
				printf("option: chunk tree %s\n", optarg);
				break;
			case '?':
			case 'h':
				printf("option: help\n");
				break;
			case GETOPT_VAL_REPAIR:
				printf("option: repair\n");
				break;
			case GETOPT_VAL_READONLY:
				printf("option: readonly\n");
				break;
			case GETOPT_VAL_INIT_CSUM:
				printf("option: init cum tree\n");
				break;
			case GETOPT_VAL_INIT_EXTENT:
				printf("option: init extent tree\n");
				break;
			case GETOPT_VAL_CHECK_CSUM:
				printf("option: check csum\n");
				break;
			case GETOPT_VAL_MODE:
				printf("option: mode %s\n", optarg);
				break;
			case GETOPT_VAL_CLEAR_SPACE_CACHE:
				printf("option: clear space cache %s\n", optarg);
				break;
			case GETOPT_VAL_FORCE:
				printf("option: force\n");
				break;
			default:
				printf("unknown option %d\n", c);
				break;
		}
	}
	return 0;
}
---


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: About more loose parameter sequence requirement

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 3644 bytes --]



On 2018年06月19日 12:34, Qu Wenruo wrote:
> 
> 
> On 2018年06月18日 20:02, David Sterba wrote:
>> On Mon, Jun 18, 2018 at 07:40:44PM +0800, Qu Wenruo wrote:
>>>
>>>
>>> On 2018年06月18日 19:34, David Sterba wrote:
>>>> On Thu, Jun 14, 2018 at 03:17:45PM +0800, Qu Wenruo wrote:
>>>>> I understand that btrfs-progs introduced restrict parameter/option order
>>>>> to distinguish global and sub-command parameter/option.
>>>>>
>>>>> However it's really annoying if one just want to append some new options
>>>>> to previous command:
>>>>>
>>>>> E.g.
>>>>> # btrfs check /dev/data/btrfs
>>>>> # !! --check-data-csum
>>>>>
>>>>> The last command will fail as current btrfs-progs doesn't allow any
>>>>> option after parameter.
>>>>>
>>>>>
>>>>> Despite the requirement to distinguish global and subcommand
>>>>> option/parameter, is there any other requirement for such restrict
>>>>> option-first-parameter-last policy?
>>>>
>>>> I'd say that it's a common and recommended pattern. Getopt is able to
>>>> reorder the parameters so mixed options and non-options are accepted,
>>>> unless POSIXLY_CORRECT (see man getopt(3)) is not set. With the more
>>>> strict requirement, 'btrfs' option parser works the same regardless of
>>>> that.
>>>
>>> But currently it doesn't work.
>>> Just as the example, btrfs check /dev/data/btrfs --check-data-sum won't
>>> work.
>>> It's different from a lot of other common programs.
>>
>> With POSIXLY_CORRECT set, it expectedly won't work. With POSIXLY_CORRECT
>> unset, it would work in general, but not for 'btrfs'.
>>
>> As this is per-application decision I find it ok, besides that I also
>> find the 'options anywhere' pattern bad. Does it really save you that
>> much time while typing the commands with new arguments? There are
>> movement shortcuts to jump by words, you get the previous command by
>> up-arrow. About the same number of keystrokes.
>>
>> New code needs to be tested, documented and maintained, that's the cost
>> I find too high for something that's convenience for people who are used
>> to some shell builtins.
>>
> 
> Well, after some testing, the result looks pretty strange on my side.
> 
> With the testing code (*), mostly just copied from check/main.c, it
> works to detect the final --check-data-csum without problem.
> 
> I'm originally planning to use '-' to make getopt_long() to keep the
> original order, but the experiment I did shows it unnecessary.
> 
> And furthermore, even changing the optstring of btrfs check
> getopt_long() with leading '-', it still doesn't work as expected to
> detect non-option parameter.
> 
> Is there anything wrong/special in btrfs-progs getopt_long() usage?

OK, indeed there is something wrong here.

Just as man page of getopt_long(3) said:
---
NOTES
       A program that scans multiple argument vectors,  or  rescans  the
       same  vector  more than once, and wants to make use of GNU exten‐
       sions such as '+' and '-' at the start of optstring,  or  changes
       the  value  of  POSIXLY_CORRECT  between scans, must reinitialize
       getopt() by resetting optind to 0, rather  than  the  traditional
       value of 1.  (Resetting to 0 forces the invocation of an internal
       initialization routine that rechecks POSIXLY_CORRECT  and  checks
       for GNU extensions in optstring.)
---

So, "btrfs check /dev/data/btrfs --check-data-csum" should work.

And the bug is, we reset @optind to 1, other than 0 when calling
getopt_long() on the new argc/argv.

I'll fix it soon.

Thanks,
Qu

> 
> Thanks,
> Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: About more loose parameter sequence requirement

[David Sterba]

On Mon, Jun 18, 2018 at 09:05:59PM +0800, Qu Wenruo wrote:
> > New code needs to be tested, documented and maintained, that's the cost
> > I find too high for something that's convenience for people who are used
> > to some shell builtins.
> 
> The biggest problem is, the behavior isn't even consistent across
> btrfs-progs.
> mkfs.btrfs accept such out-of-order parameters while btrfs not.
> 
> And most common tools, like commands provided by coretutil, they don't
> care about the order.
> The only daily exception is 'scp', which I found it pretty unhandy.
> 
> And just as Paul and Hugo, I think there are quite some users preferring
> out-of-order parameter/options.

Because of the feedback and interest of the relaxed mixed
option/argument syntax, I don't object anymore.

Re: About more loose parameter sequence requirement

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1106 bytes --]



On 2018年06月19日 22:47, David Sterba wrote:
> On Mon, Jun 18, 2018 at 09:05:59PM +0800, Qu Wenruo wrote:
>>> New code needs to be tested, documented and maintained, that's the cost
>>> I find too high for something that's convenience for people who are used
>>> to some shell builtins.
>>
>> The biggest problem is, the behavior isn't even consistent across
>> btrfs-progs.
>> mkfs.btrfs accept such out-of-order parameters while btrfs not.
>>
>> And most common tools, like commands provided by coretutil, they don't
>> care about the order.
>> The only daily exception is 'scp', which I found it pretty unhandy.
>>
>> And just as Paul and Hugo, I think there are quite some users preferring
>> out-of-order parameter/options.
> 
> Because of the feedback and interest of the relaxed mixed
> option/argument syntax, I don't object anymore.

And in fact, the behavior of btrfs is caused by @optind wrongly initialized.
The fix is just one line (along some comments).

https://patchwork.kernel.org/patch/10473173/

So no or little pressure on maintenance.

Thanks,
Qu


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: About more loose parameter sequence requirement

[Qu Wenruo]

[-- Attachment #1.1: Type: text/plain, Size: 1496 bytes --]



On 2018年06月20日 07:18, Qu Wenruo wrote:
> 
> 
> On 2018年06月19日 22:47, David Sterba wrote:
>> On Mon, Jun 18, 2018 at 09:05:59PM +0800, Qu Wenruo wrote:
>>>> New code needs to be tested, documented and maintained, that's the cost
>>>> I find too high for something that's convenience for people who are used
>>>> to some shell builtins.
>>>
>>> The biggest problem is, the behavior isn't even consistent across
>>> btrfs-progs.
>>> mkfs.btrfs accept such out-of-order parameters while btrfs not.
>>>
>>> And most common tools, like commands provided by coretutil, they don't
>>> care about the order.
>>> The only daily exception is 'scp', which I found it pretty unhandy.
>>>
>>> And just as Paul and Hugo, I think there are quite some users preferring
>>> out-of-order parameter/options.
>>
>> Because of the feedback and interest of the relaxed mixed
>> option/argument syntax, I don't object anymore.
> 
> And in fact, the behavior of btrfs is caused by @optind wrongly initialized.
> The fix is just one line (along some comments).
> 
> https://patchwork.kernel.org/patch/10473173/
> 
> So no or little pressure on maintenance.

I'm totally wrong.
Since there are cases we call check_argc_*() directly using @optind,
reinitialize it to 0 would cause them to fail.

I'll update the patch to manually reset optind to 0 only for cases we
call getopt().
And indeed, it adds extra maintenance effort.

Thanks,
Qu

> 
> Thanks,
> Qu
> 


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: About more loose parameter sequence requirement

[Hans van Kranenburg]

On 06/18/2018 03:05 PM, Qu Wenruo wrote:
> 
> 
> On 2018年06月18日 20:02, David Sterba wrote:
>> On Mon, Jun 18, 2018 at 07:40:44PM +0800, Qu Wenruo wrote:
>>>
>>>
>>> On 2018年06月18日 19:34, David Sterba wrote:
>>>> On Thu, Jun 14, 2018 at 03:17:45PM +0800, Qu Wenruo wrote:
>>>>> I understand that btrfs-progs introduced restrict parameter/option order
>>>>> to distinguish global and sub-command parameter/option.
>>>>>
>>>>> However it's really annoying if one just want to append some new options
>>>>> to previous command:
>>>>>
>>>>> E.g.
>>>>> # btrfs check /dev/data/btrfs
>>>>> # !! --check-data-csum
>>>>>
>>>>> The last command will fail as current btrfs-progs doesn't allow any
>>>>> option after parameter.
>>>>>
>>>>>
>>>>> Despite the requirement to distinguish global and subcommand
>>>>> option/parameter, is there any other requirement for such restrict
>>>>> option-first-parameter-last policy?
>>>>
>>>> I'd say that it's a common and recommended pattern. Getopt is able to
>>>> reorder the parameters so mixed options and non-options are accepted,
>>>> unless POSIXLY_CORRECT (see man getopt(3)) is not set. With the more
>>>> strict requirement, 'btrfs' option parser works the same regardless of
>>>> that.
>>>
>>> But currently it doesn't work.
>>> Just as the example, btrfs check /dev/data/btrfs --check-data-sum won't
>>> work.
>>> It's different from a lot of other common programs.
>>
>> With POSIXLY_CORRECT set, it expectedly won't work. With POSIXLY_CORRECT
>> unset, it would work in general, but not for 'btrfs'.
>>
>> As this is per-application decision I find it ok, besides that I also
>> find the 'options anywhere' pattern bad. Does it really save you that
>> much time while typing the commands with new arguments?
> 
> Personally speaking, yes.
> 
>> There are
>> movement shortcuts to jump by words, you get the previous command by
>> up-arrow. About the same number of keystrokes.
> 
> Not really, normally just "!! <new options>", where I don't even need to
> move my fingers to arrow keys.
> (I'm all ears about extra bash shortcuts on this)

It's called `set -o vi` ;-]

If you remap capslock to be escape, there's not much moving around of
fingers going on. CAPS-k-b-b-a and type your new option. No reaching to
shift and 1 necessary at all.

:]

>>
>> New code needs to be tested, documented and maintained, that's the cost
>> I find too high for something that's convenience for people who are used
>> to some shell builtins.
> 
> The biggest problem is, the behavior isn't even consistent across
> btrfs-progs.
> mkfs.btrfs accept such out-of-order parameters while btrfs not.
> 
> And most common tools, like commands provided by coretutil, they don't
> care about the order.
> The only daily exception is 'scp', which I found it pretty unhandy.
> 
> And just as Paul and Hugo, I think there are quite some users preferring
> out-of-order parameter/options.
> 
> 
> I also understand the maintenance burden, but at least let's try if
> there are better solution for this.
> 
> Thanks,
> Qu
> 


-- 
Hans van Kranenburg