This patch helps make the short options displayed in the help message be consistant with the description in https://llvm.org/docs/CommandGuide/dsymutil.html
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
I'm not a dsymutil user or developer, so whilst your change makes sense, I'm not sure if I'm best placed to review it. I've added a couple more reviewers who have recently touched dsymutil.
LGTM.
Before I converted option parsing to libOpt it was using cl::opt which does not differentiate between the two. Unfortunately we have users that have to come to rely on this. I think it's a small enough price to pay to keep them around and not break their workflows.
Not sure how applicable this is here, but I wanted to point out that we do have a test specifically for the help text llvm/test/tools/dsymutil/cmdline.test. Perhaps this patch can be tested there.
Hi @aprantl
Sorry for the late response. I've checked cmdline.test, and these options are checked in this way.
dsymutil --help | FileCheck %s --check-prefix=HELP HELP: -option1 HELP: -option2
However, -option matches both --option and -option. I've also checked other tools' cmdline.test/help.test, which are similar to dsymutil's. I guess we should use regex patterns to exactly match what we want? Do you have any suggestion?
Assuming we want only "--option1" to be matched, why not simply change the text to --option1? If we want to allow either version, then -option1 is fine, or you could be more specific with {{-?}}-option1. Finally, you could also ensure there'se a space before the option by doing {{ }}--option1 or even {{ -?}}-option1.
Sorry for the misleading, in this patch, I changed some long options (--o, --y) to (-o and -y), however, for example, pattern "-o" matches both --o and -o. I am wondering, is there any way to match exactly -o? I've checked other LLVM tools, and it seems that there is no such pattern?
Ah, okay. FileCheck has "--match-full-lines" as one option, but it's probably not what you want in this case, as there will be other text on the same line that you don't want. Assuming there is whitespace before and after the option name, you can adapt the second-to last of my suggestions above to {{ }}-o{{ }} or even {{ -o }}. Spaces within the regex will be forcibly matched which should solve your problems. You can also use ^ and $ for regex markers for the start* and end of a line.
- "start" here means the end of the previous match, if on the same line as the current one, or the start of the line otherwise.
Thanks you @jhenderson and @aprantl, it helps a lot!
Addressed comments.
- Update cmdline.test to test this change.