<div dir="ltr"><div>Thanks for you feedback.</div>I agree, the test i've written allow to make regression tests for the issues.<div><br></div><div>Testing <span style="font-size:13.3333339691162px;font-family:arial,sans-serif">AnnotatingParser</span><font face="arial, sans-serif"> would have removed formatting consideration from the parser tests which should have produced simpler and more targeted and stable tests for it.</font></div><div>Since the formatting is dependent on its output, separating these 2 considerations seemed to make sense.</div><div><br></div><div><div><span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:13.3333339691162px">If you think that it may be worthwhile, I could try to code a small working </span><span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:13.3333339691162px">example</span><span style="color:rgb(80,0,80);font-family:arial,sans-serif;font-size:13.3333339691162px"> of </span><span style="font-size:13.3333339691162px;color:rgb(80,0,80);font-family:arial,sans-serif">test for a stand alone </span><span style="font-size:13.3333339691162px;font-family:arial,sans-serif">AnnotatingParser to see whether it would be of use to you.</span></div></div><div><span style="font-size:13.3333339691162px;font-family:arial,sans-serif">Otherwise I'll carry on trying to fix these issues.</span></div><div><br></div><div>Best Regards,</div><div>JP</div><div class="gmail_extra"><br><div class="gmail_quote">2014-09-21 20:12 GMT+01:00 Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On Sun, Sep 21, 2014 at 7:33 PM, Jean-philippe Dufraigne <span dir="ltr"><<a href="mailto:j.dufraigne@gmail.com" target="_blank">j.dufraigne@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div>Hello Daniel,</div><div><br></div><div>I was looking at a formatting issue with "std::function<void(int, int)> callback;". "(int, int)" is understood as a C-style cast and so the spacing follow this understanding.</div><div><br></div><div>While trying to address this, I found that the existing test for member function reference qualifier relied on the confusion. it results in inconsistent format when a name is added thought (see last 2 lines of my test function)</div><div><br></div><div>This is not really dependent on the different style, and it would seem unit test for AnnotatingParser would help when updating it.</div><div><br></div><div>Would it make sense to move AnnotatingParser in its own files and then start adding unit tests for it?</div></div></blockquote><div><br></div></span><div>I don't think so. I like the unit tests as they are, testing the format of small snippets of actual code. Directly testing the AnnotatingParser feels a bit like testing implementation details. Also, the test you have written shows that you can test exactly what you want to test with the existing test infrastructure, or am I missing something?</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><span class=""><div dir="ltr"><div>I would think that we would want to split the class in its .h and .cpp file.</div><div>I'd be happy to provide a patch for this, though reviewing this kind of changes is often tricky as diffs can be unhelpful, do you have any preference on how it is done?</div><div><br></div><div>The test function that shows the issues:</div><div><br></div><div>TEST_F(FormatTest, ConfigurableSpacesInParenthesesTryThings) {</div><div> FormatStyle Spaces = getLLVMStyle();</div><div><br></div><div> Spaces.SpacesInParentheses = true;</div><div> verifyFormat("void function( int, int );", Spaces);</div><div> verifyFormat("std::function<void(int, int)> callback;", Spaces); // inconsistent</div><div> verifyFormat("#define x ( (int)-1 )", Spaces);</div><div><br></div><div> Spaces.SpacesInParentheses = false;</div><div> Spaces.SpacesInCStyleCastParentheses = true;</div><div> verifyFormat("void function(int, int);", Spaces);</div><div> verifyFormat("std::function<void( int, int )> callback;", Spaces); // inconsistent</div><div> verifyFormat("#define x (( int )-1)", Spaces);</div><div><br></div><div> verifyFormat("Deleted &operator=(const Deleted &)& = default;");</div><div> verifyFormat("Deleted &operator=(const Deleted &other) & = default;"); // inconsitent</div><div>}</div><div><br></div><div>Thanks,</div><div>JP</div></div>
<br></span>_______________________________________________<br>
cfe-dev mailing list<br>
<a href="mailto:cfe-dev@cs.uiuc.edu" target="_blank">cfe-dev@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-dev</a><br>
<br></blockquote></div><br></div></div>
</blockquote></div><br></div></div>