[PATCH] Add support for Unicode in Fix-its hints

Jordan Rose jordan_rose at apple.com
Thu Jun 6 17:00:56 PDT 2013


On Jun 6, 2013, at 16:55 , Sukolsak Sakshuwong <sukolsak at gmail.com> wrote:

> Hi Jordan,
> 
> Thank you very much for the comments.
> 
> On Thu, Jun 6, 2013 at 9:48 AM, Jordan Rose <jordan_rose at apple.com> wrote:
>> Please add more tests as well. Tests where there are multiple fix-its on the same line seem like they'd be really important, though I can't think of any off-hand that involve identifiers. You should also include at least one test to show that the fix-it still lines up properly with the replacement range.
> 
> I was thinking about that too, but I couldn't find any test case that
> would produce multiple fix-it hints on the same line and allow me to
> use arbitrary Unicode string before the last hint. I tested it
> internally by hard-coding
> "Hints.push_back(FixItHint::CreateInsertion(Location, UnicodeString))"
> into the code.
> 
> Even if we are able to do that, I am also not sure how can check the
> location of printed fix-its because different systems will render
> Unicode characters differently, as commented in
> test/FixIt/fixit-unicode.c.
> 
> Do you have any recommendations on how I should go about this?

Well, for the single fix-it case, you can check the line up to the Unicode character (the spaces at the very least). You're right that the mulitple fix-it case would be tricky.

I can't think of a multiple fix-it test case either, so I guess that shouldn't block your patch. By inspection, the code looks like it's doing the right thing -- the only place where columns and bytes are being mixed is where the columns are known to be ' '-filled.

With caret diagnostic tests for cases where the Unicode characters are at the start, middle, and end of the word, I think this can go in. Will you need me to commit for you?

Jordan



More information about the cfe-commits mailing list