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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
