[PATCH] D103250: [llvm-dwarfdump][test] Add missing dedicated tests for some options

[Xing GUO via Phabricator via llvm-commits]

Higuoxing added inline comments.


================
Comment at: llvm/tools/llvm-dwarfdump/llvm-dwarfdump.cpp:638
   ToolOutputFile OutputFile(OutputFilename, EC, sys::fs::OF_TextWithCRLF);
-  error("Unable to open output file" + OutputFilename, EC);
+  error("Unable to open output file " + OutputFilename, EC);
   // Don't remove output file if we exit with an error.
----------------
gbreynoo wrote:
> Higuoxing wrote:
> > jhenderson wrote:
> > > Whilst you're changing this message, I'd also change `Unable` to `unable`, in line with the LLVM error message style policy.
> > I think it might be good to surround file names with single quotes `'`. e.g.
> > 
> > ```
> > Unable to open output file '/path/to/file': permission denied
> > ```
> I'd have to check if OutputFilename is empty or an error like `unable to open output file  '': no such file or directory` can be output which looks strange IMHO . Would it better to leave it as is?
Sounds reasonable to me. Thanks!


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

https://reviews.llvm.org/D103250