mergePersistentFlags in Traverse

[gmwxio]

more elegant fix than #615

[eparis]

I think this is also wrong. @dnephin probably needs to come in an look at things. It appears we re-wrote (broken) flag parsing code in func (c *Command) Traverse(). I'll try to comment more this afternoon. But your example program is great!

https://github.com/wxio/cobra/tree/pr615test/tests

[dnephin]

This fix is correct. The related logic for non-traversal is

cobra/command.go

Lines 459 to 463 in 0c34d16

	func stripFlags(args []string, c *Command) []string {
	if len(args) == 0 {
	return args
	}
	c.mergePersistentFlags()

Traversal uses similar logic to stripFlags(), but it's not the same. Some of the conditions in the switch are the same, so I can factor those out to shared helpers, but otherwise the behaviour is different.

I'll open a PR to share more of the logic between stripFlags() and Traverse().

[dnephin]

@wxio thank you for the bug report, the test case, and the fix. I've opened #617 which includes your commit here. It also adds the test case as an automated test to the test suite, and refactors the code a bit to share more.

[eparis]

Ok, I'm on board that this fixes one of the bugs in Traverse. I am good either way If we want to merge this and then iterate on more fixes in another PR, or if @dnephin wants to try to do it all in one shot in another PR.

[dnephin]

I'd like to get #617 merged so there is a test for this fix as well. I can open an issue for the other problem.

[spf13]

@eparis is this still worth merging?

[github-actions]

This PR is being marked as stale due to a long period of inactivity

[wfernandes]

@jharshman Seems like we can close this PR in favor of #617 since that PR also contains @wxio's commit.

[jharshman]

@wfernandes agreed