[PATCH] D56513: [llvm-objcopy][NFC] Consistenly use two dashes for flags in tests.

James Henderson via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 10 01:34:41 PST 2019


jhenderson added a comment.

What's most important to me is that we are consistent, primarily within individual files, so that it looks somewhat professional ;-). I do agree that if we want to support single-dash options, we should test both that and double-dash explicitly, much like we test both short and long options. This would probably belong in the "basic" test for each option.

Aside from the fact that LLVM tools traditionally allow it, single-dash long-form arguments seems like a recipe for disaster when mixing in the ability to combine single-letter options into a single argument, and I'm not convinced it should be allowed at all: I haven't tested in detail, but at least for the GNU Win32 version of grep that I have easily to hand, single-dash followed by lots of letters causes issues:

  C:\Work>grep -recursive blah .
  grep: blah: No such file or directory
  
  C:\Work>grep -recursive "jnjhj" blah
  grep: jnjhj: No such file or directory
  grep: blah: No such file or directory
  
  C:\Work>grep --recursive "jnjhj" blah
  grep: blah: No such file or directory

I don't have objdump or others immediately to hand to try those out.



================
Comment at: test/tools/llvm-objcopy/ELF/help-message.test:1
 # RUN: llvm-objcopy --help | FileCheck --check-prefix=OBJCOPY-USAGE %s
 # RUN: not llvm-objcopy 2>&1 | FileCheck --check-prefix=OBJCOPY-USAGE %s
----------------
This file in particular should test both single and double dash, because the semantics of single dash are slightly different in general terms (not necessarily in llvm-objcopy, although perhaps they should be), owing to short options being combinable into a single arg.


================
Comment at: test/tools/llvm-objcopy/ELF/objcopy-version.test:1
-# RUN: llvm-objcopy -version | FileCheck %s
+# RUN: llvm-objcopy --version | FileCheck %s
 # RUN: llvm-objcopy --version | FileCheck %s
----------------
False one here. This test is actually an example of what I think we should aim to emulate (i.e. short, long with single dash and long with double dash).


================
Comment at: test/tools/llvm-objcopy/ELF/strip-version.test:1
-# RUN: llvm-strip -version | FileCheck %s
+# RUN: llvm-strip --version | FileCheck %s
 # RUN: llvm-strip --version | FileCheck %s
----------------
Same as above, don't make this change here.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56513/new/

https://reviews.llvm.org/D56513





More information about the llvm-commits mailing list