<div dir="ltr">On Fri, Aug 16, 2013 at 7:32 PM, Edwin Vane <span dir="ltr"><<a href="mailto:edwin.vane@intel.com" target="_blank">edwin.vane@intel.com</a>></span> wrote:<br><div class="gmail_extra"><div class="gmail_quote">

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div><br>
<br>
================<br>
Comment at: include/clang/Tooling/ReplacementsYaml.h:28<br>
@@ +27,3 @@<br>
+/// \brief The top-level YAML document that contains all Replacements.<br>
+struct ReplacementsDocument {<br>
+  /// A freeform chunk of text to describe the context of the replacements in<br>
----------------<br>
</div><div>Manuel Klimek wrote:<br>
> I'd vote against calling this "Document". To me, a document is something I edit :)<br>
> Perhaps we can just call it yaml::Replacement? or YAMLReplacement?<br>
</div>I'm using YAML nomenclature here. A document is everything between --- and ... in a YAML file where a YAML file can have multiple documents. Given that reasoning do you still feel strongly about renaming it?<br>

</blockquote><div><br></div><div>I figured that that's the case. My question is, whether we really want the strange names of the YAML domain to blend in here...</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div>
================<br>
Comment at: include/clang/Tooling/ReplacementsYaml.h:31<br>
@@ +30,3 @@<br>
+  /// this document.<br>
+  std::string Context;<br>
+<br>
----------------<br>
</div><div>Manuel Klimek wrote:<br>
> I'd put some stronger idea of what we want here into the comment:<br>
> // A chunk of text that gives context on how the replacement was generated (for example,<br>
> // the translation unit parsed, the location of a template instantiation, etc).<br>
> // This will be used for diagnostic purposes if the application of a replacement fails.<br>
><br>
> Furthermore, why don't we have one of those per replacement? If it's useful enough, I think we should really put it into the Replacement class as an optional field, and provide nice libs to populate it from a clang-tool.<br>


><br>
> Also, I'd perhaps call it DiagnosticInfo (but I don't really feel strongly here, just throwing out an idea).<br>
</div>If we put Context into Replacement I'd definitely want to name it something that implies it's an annotation and not part of Replacement comparison operators. Perhaps "metadata" a la LLVM?<br></blockquote>
<div><br></div><div>I think "metadata" basically tells me nothing. I'd want to make sure it's intended to be used for diagnostics, and that it should include "how" the replacement was generated.</div>
</div></div></div>