CLI Option reports arguments incorrectly on toString()

• Key: CLI-141
• Type: Bug
• Status: Closed
• Resolution: Fixed
• Priority: Minor
• Assignee: Unassigned
• Reporter: Henning Schmiedehausen
• Component/s: CLI-1.x
• Affects Version/s: 1.1
• Fix Version/s: 1.2

Description

The Option class contains some logic around the "hasArg" boolean that seems to be cruft and no longer works correctly. There is no need for this boolean anymore and its only usage could be replaced with hasArg() / hasArgs() calls.

Comments

[Henning Schmiedehausen]

Also contains serializable patch, apply with care.

[Brian Egge]

I agree with the hasArg change. My preference for serializable classes is to use the number which would be generated by serialver. This way, if someone is already serializing this class, it won't break their code if they don't upgrade both the client and server at the same time.

Other people prefer to start a 1L and increment each time the public interface changes. I don't know if Apache has a standard one way or the other.

[Henning Schmiedehausen]

You can not serialize the class currently. It is not serializable.

It has no version, because it needs no version. I don't know anything about a serialization standard @ Apache, but I consider 1L and incrementing to be more readable.

I don't really care about the version, as long as the classes become serializable. I have some time this weekend, so I could apply the patches, unless someone objects strongly.

[Henri Yandell]

Apply away

[Henning Schmiedehausen]

Will do.

[Henri Yandell]

Patch is wonky btw - it fails to apply against the branch, and when I look at it by hand, you've missed one of the hasArg usages.

[Bjorn Townsend]

Cleaned up the patch a little bit. It applies to the 1.x branch for me without any problems.

The hasArg instance that Henri thought Henning missed is not an issue – it's just a local variable. +1 to adopting this.

[Henri Yandell]

Patch added: r596140.