[PATCH] D86137: Add ignore-unknown-options flag to clang-format.

[Joachim Meyer via Phabricator via cfe-commits]

fodinabor added a subscriber: grimar.
fodinabor added a comment.

Thank you so far for the feedback!
maybe you can give further guidance on the comments on the comments :)

As of the git history it seems that @grimar did some work on YAML error handling..



================
Comment at: clang/tools/clang-format/ClangFormat.cpp:108
+static cl::opt<bool>
+    IgnoreUnkownOptions("ignore-unknown-options",
+                        cl::desc("If set, unknown format options are ignored."),
----------------
MyDeveloperDay wrote:
> feels like a mouthful is there nothing shorter we could use?  -Wignore (or something)
hmm... `-Wunknown`
but the `-W` does not really make it clear that the default "errors" should now be treated as warnings instead. From compiler conventions, I'd expect the `-W` to enable a warning ... 

and something like `-Wno-error=unknown` is not really shorter...


================
Comment at: llvm/include/llvm/Support/YAMLTraits.h:1520
   bool                                ScalarMatchFound = false;
+  bool IgnoreUnkown = false;
 };
----------------
MyDeveloperDay wrote:
> is this clang-formatted?
the patch is... the original alignment of the members is not. (also see the `CurrentNode`'s formatting).
not sure what to do in this case, as the whole file seems rather unformatted?


================
Comment at: llvm/lib/Support/YAMLTraits.cpp:199
+  if (IgnoreUnkown)
+    return;
   for (const auto &NN : MN->Mapping) {
----------------
MyDeveloperDay wrote:
> do we want to flat out ignore or just report but not fatally. (just a thought) silent failures are hard to diagnose
true.. don't know what's the best option?

keep it as a printed out error and just don't return an error code on exit? This option would make it a clang-format only change, but feels really dirty.

Otherwise I'd have to dig my way through to `llvm::yaml::Stream::printError` (or maybe rather add a `printWarning`) to conditionally change the message type for the ignore case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86137