[PATCH] D52999: [FileCheck] Annotate input dump (1/7)

[Joel E. Denny via Phabricator via llvm-commits]

jdenny updated this revision to Diff 176895.
jdenny edited the summary of this revision.
jdenny set the repository for this revision to rL LLVM.
jdenny added a comment.

Made changes suggested by probinson:

- In docs, say `-dump-input-on-failure` is deprecated in favor of `-dump-input=fail` not `-dump-input`.
- Use `cl::values` (enum) for `-dump-input` value.
- In annotation description, don't document space after line number.
- Don't bother collecting diagnostics if `-dump-input=never`.
- Instead of leaving `-dump-input-on-failure` as a separate feature, make it an alias for `-dump-input=fail` but only when `-dump-input` is not otherwise specified.
- Move annotation description to a `-dump-input=help` option.

I also removed the detailed description of the currently enabled
markers.  In place of that, I added a few notes to the brief
description that preceded it.  Here's why:

- The reviews so far have led me to believe that having two descriptions was more confusing than helpful.  George questioned whether the marker descriptions should be repeated (D52999#1260736 <https://reviews.llvm.org/D52999#1260736>). George didn't understand the short version of the `!~~` description (D52999#1260736 <https://reviews.llvm.org/D52999#1260736>).  Paul questioned why `!~~` should represent both a discarded match and actual errors (D53898#1317412 <https://reviews.llvm.org/D53898#1317412>).  (Perhaps we really need an additional marker, but my assumption is that the docs just aren't clear.)
- The detailed description added complexity to multiple parts of the existing implementation (MatchType, MatchTypeStyle, and of course DumpInputAnnotationHelp).
- Previously the detailed description changed based on whether color was enabled and based on the verbosity level.  That seemed to make sense when the description printed with the dump.  Now it requires the user to be sure to specify certain options the same when requesting help as when requesting the dump.  The options in the latter case might be buried in a `FILECHECK_OPTS` in a bot, IDE, or script, so getting the options right for help could be error-prone.

If you feel that part of this change makes things worse, I can revert
it.  Hopefully it's a helpful simplification and can be tweaked to
address any further problems.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D52999

Files:
  llvm/docs/CommandGuide/FileCheck.rst
  llvm/include/llvm/Support/FileCheck.h
  llvm/lib/Support/FileCheck.cpp
  llvm/test/FileCheck/dump-input-annotations.txt
  llvm/test/FileCheck/dump-input-enable.txt
  llvm/test/FileCheck/no-check-file.txt
  llvm/test/FileCheck/verbose_mode.txt
  llvm/utils/FileCheck/FileCheck.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D52999.176895.patch
Type: text/x-patch
Size: 40267 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20181206/9cca3c72/attachment.bin>