Re: About more loose parameter sequence requirement
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. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: About more loose parameter sequence requirement
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 > signature.asc Description: OpenPGP digital signature
Re: About more loose parameter sequence requirement
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. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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). 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 signature.asc Description: Digital signature
RE: About more loose parameter sequence requirement
> -Original Message- > From: linux-btrfs-ow...@vger.kernel.org ow...@vger.kernel.org> On Behalf Of Hugo Mills > Sent: Monday, 18 June 2018 9:44 PM > To: dste...@suse.cz; Qu Wenruo ; linux- > bt...@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. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: About more loose parameter sequence requirement
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 "!! ", 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 signature.asc Description: OpenPGP digital signature
Re: About more loose parameter sequence requirement
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
#include
#include
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':
Re: About more loose parameter sequence requirement
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 signature.asc Description: OpenPGP digital signature
Re: About more loose parameter sequence requirement
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. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: About more loose parameter sequence requirement
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 signature.asc Description: OpenPGP digital signature
Re: About more loose parameter sequence requirement
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 > signature.asc Description: OpenPGP digital signature
Re: About more loose parameter sequence requirement
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 "!! ", 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 -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
