-
Notifications
You must be signed in to change notification settings - Fork 0
adding new version of find which customizes the behaviour using flag … #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
chapter5/findwithflag.go
Outdated
|
||
func main() { | ||
|
||
minusS := flag.Bool("s", false, "Sockets") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why minusS
? what does that mean?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
similarly for all the other flags
chapter5/findwithflag.go
Outdated
flags := flag.Args() | ||
|
||
printAll := false | ||
if *minusS && *minusP && *minusSL && *minusD && *minusF { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is making the logic complex, why not create a list of all and then check for each flag and remove values one by one
chapter5/findwithflag.go
Outdated
printAll = true | ||
} | ||
|
||
if !(*minusS || *minusP || *minusSL || *minusD || *minusF) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you follow above comment, this will not be needed
printAll = true | ||
} | ||
|
||
if len(flags) == 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why check for len? if you have to treat flags as positional arguments then wh use flag package at all? then you can use os.ARGS only
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since we are using flag for flags anyways and it has abstration over os.Args which does the same job
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You mean using flag you can get positional as well as names arguments at the same time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes using flag.Args we can take positional arguments only difference between this and os.Args is its indexing starts from zero
} | ||
|
||
fileInfo, _ = os.Lstat(path) | ||
if fileInfo.Mode()&os.ModeSymlink != 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there are formatting issues, please use goImports on save in sublime. Google it how to do that
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added goimport on save in sublime
chapter5/findwithflag.go
Outdated
|
||
fileInfo, _ = os.Lstat(path) | ||
if fileInfo.Mode()&os.ModeSymlink != 0 { | ||
if *minusSL { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please rename flag names so that I can understand what are they for.
} | ||
} | ||
|
||
if fileInfo.Mode()&os.ModeNamedPipe != 0 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add comments above each such check and explain what are you trying to do
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
…package