go-ethereum icon indicating copy to clipboard operation
go-ethereum copied to clipboard

internal/flags: fix issue with stringslice migration

Open holiman opened this issue 2 years ago • 3 comments

This PR fixes an issue related to how subcommands are hoisted into the global scope.

Here's an example app: https://gist.github.com/holiman/5ed50fc9dcf44fd661abdb91a59137fe#file-main-go . This has a stringslice argument, and shows the corruption that happens when that stringslice is migrated across scopes:

[user@work cmdtest]$ ./cmdtest  --header aa --header bb --url foo connect
invoked via subcommand
  ctx.IsSet(HttpHeaderFlag.Name): true
    0: [aa bb]
  ctx.IsSet(UrlFlag.Name): true
    foo

The value is migrated as the literal string [aa bb], which is the string-equivalent of the stringslice. Unfortunately, the backing type, StringSlice cannot convert this back to a proper stringslice -- it expects input to be on the form aa,bb.

This PR fixes it by detecting whether a given type is indeed a stringslice, and then uses a different string-representation than the default. This works nicely with the backing-type, which is then set correctly.

[user@work cmdtest]$ ./cmdtest  --header aa --header bb --url foo connect
invoked via subcommand
  ctx.IsSet(HttpHeaderFlag.Name): true
    0: aa
    1: bb
  ctx.IsSet(UrlFlag.Name): true
    foo

holiman avatar Sep 20 '22 10:09 holiman

The initial fix did not quite handle flag aliases. There's a special quirk around those, which is now handled too.

holiman avatar Sep 20 '22 11:09 holiman

Idea seems ok, just need to review/test offline

karalabe avatar Sep 20 '22 12:09 karalabe

You can use https://gist.github.com/holiman/5ed50fc9dcf44fd661abdb91a59137fe#file-main-go to play around with how it works. Updated now to also use alias

holiman avatar Sep 20 '22 13:09 holiman