[PATCH] D44598: [lit] Report line number for failed RUN command

[Dan Liew via Phabricator via llvm-commits]

delcypher added inline comments.


================
Comment at: utils/lit/lit/TestRunner.py:792
 
+    if cmd.commands[0].args[0] == ':':
+        if len(cmd.commands) != 1:
----------------
jdenny wrote:
> delcypher wrote:
> > The null command is pretty esoteric but occasionally useful in normal shell script. What's the reason it's not supported?
> > I guess we don't normally need it and it's a waste of time to implement it for the internal shell?
> That would be my guess.  Do you object to implementing it for this purpose?
I'm fine with you not implementing the null command. I was just curious. You could perhaps leave a `TODO` or `FIXME` commenting that we don't need to support it right now.


================
Comment at: utils/lit/lit/TestRunner.py:1367
                 output = []
+            line = ': \'' + keyword + ' at line ' + str(line_number) + \
+                   '\'; ' + line
----------------
jdenny wrote:
> delcypher wrote:
> > I think this would be more readable as
> > 
> > ```
> > line = ": '{keyword} at line {line}'; {real_command}".format(
> >   keyword=keyword,
> >   line=line,
> >   real_command=line)
> > ```
> > 
> > Or something similar. There's no reason for the `\'` if you use `"` quotes instead.
> Sure, I'll change that and the other occurrence you pointed out.
Great. I also much prefer use of python's string format function as in the example I gave but I'll leave that up to you.


https://reviews.llvm.org/D44598