projectdiscovery / goflags

A go flag wrapper with convenience helpers
MIT License
85 stars 15 forks source link

`DynamicVar` did unexpected works #187

Closed XiaoliChan closed 6 months ago

XiaoliChan commented 6 months ago

DynamicVar & DynamicVarP is not working

ENV:

Code: https://github.com/projectdiscovery/goflags/blob/main/examples/basic/main.go

Issues details:

RamanaReddy0M commented 6 months ago

@XiaoliChan it will take default value into action when we mention flag and doesn't provide any value. ex:

 ✗ ./basic       
Got Severity: [high]
Dynamic Values Output
title size: 0
target: 
hashes: []
➜  basic git:(main) ✗ ./basic -title
Got Severity: [high]
Dynamic Values Output
title size: 50
target: 
hashes: []
XiaoliChan commented 6 months ago

@RamanaReddy0M Thanks for your reply.

I see, but it looks like weird

┌──(xiaoli㉿kali)-[/tmp/goflags/examples/basic]
└─$ ./main 
Got Severity: [high]
Dynamic Values Output
title size: 0
target: 
hashes: []

┌──(xiaoli㉿kali)-[/tmp/goflags/examples/basic]
└─$ ./main -title
Got Severity: [high]
Dynamic Values Output
title size: 50
target: 
hashes: []

In the user's eyes, it should look like other flags, provide a default value to the variable.

The second thing is even I provide value to the flag, it looks like a bool type variable.

┌──(xiaoli㉿kali)-[/tmp/goflags/examples/basic]
└─$ ./main -title 10
Got Severity: [high]
Dynamic Values Output
title size: 50
target: 
hashes: []

Seems a bit counterintuitive

RamanaReddy0M commented 6 months ago

@XiaoliChan

$ ./main -title=10

This is how you are supposed to pass the value for the flags created by DynamicVar.

The DynamicVar serves the purpose of flag without value(takes default value) or with value which is missing in go standard library and flag=xxx is the limitation we had here.

If this is not the case you can use regular IntVar for the consistency.

XiaoliChan commented 6 months ago

@RamanaReddy0M Thanks for providing more details about DynamicVar

The reason why I asking this, because I want to use float type value, but looks like it's not support

RamanaReddy0M commented 6 months ago
var tmp float64
flagSet.CreateGroup("Dynmaic", "Dynamic",
        flagSet.DynamicVar(&tmp, "tmp", 38.0, "temperature"),
    )

it support float as well.

XiaoliChan commented 6 months ago

Oh, I also need to set it as default XD

RamanaReddy0M commented 6 months ago

@XiaoliChan seems resolved closing the issue.