[PATCH] D93341: [FileCheck] Do not skip end of line in diagnostics

[Thomas Preud'homme via Phabricator via llvm-commits]

thopre added inline comments.


================
Comment at: llvm/utils/FileCheck/FileCheck.cpp:666
         if (*InputFilePtr == '\n')
           Newline = true;
         else
----------------
jdenny wrote:
> thopre wrote:
> > jdenny wrote:
> > > There should be a `COS << ' ';` here.
> > I'm not sure this is a good idea because people might wonder why there's a space in the input. Right now the shown input matches the actual input in that regards.
> > 
> > I don't know what to think about the ~ marks though. How do annotations even work for a single CHECK directive that matches several lines (e.g. CHECK: foo{{[\n]}}bar)?
> > I'm not sure this is a good idea because people might wonder why there's a space in the input. Right now the shown input matches the actual input in that regards.
> 
> Good point.  I was trying to ensure that unmatched text highlighting would also apply to newlines, but maybe it's not worth it.
> 
> > I don't know what to think about the ~ marks though. How do annotations even work for a single CHECK directive that matches several lines (e.g. CHECK: foo{{[\n]}}bar)?
> 
> Search ranges and match ranges are the same in that regard:
> 
> ```
> <<<<<<
>          1: FOO 
> check:1     ^~~~
>          2: BAR 
> check:1     ~~~
> >>>>>>
> ```
> 
> I do not think there should be an inconsistency between whether newlines are annotated at the beginning of a range (`^`, `X`, etc.) and whether they're annotated somewhere within a range (`~`), especially at the end of a range.
> 
> For example, where `$[[:space:]]` is a little known way to exclusively match a newline (the `\n` in your patch summary doesn't work for me):
> 
> ```
> $ cat check
> CHECK: {{FOO[[:space:]]*}}
> CHECK: {{$[[:space:]]BAR}}
> 
> $ FileCheck -vv check < input  |& tail -6
> <<<<<<
>          1: FOO
> check:1     ^~~
>          2: BAR
> check:2     X~~ error: no match found
> >>>>>>
> ```
> 
> Why did the second directive skip the newline on line 1?  An extra tilde at the end of line 1 would make it clearer.
Sorry for the late reply. I'm not sure how to read your answer. Do you agree about not putting a space at the end of each input line? I'm not sure what you refer by "unmatched text highlighting".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93341