This replaces tabs with 2 spaces.
Diff Detail
- Repository
- rL LLVM
- Build Status
Buildable 28794 Build 28793: arc lint + arc unit
Event Timeline
Isn't this because there are tabs in the output of these tools? In those cases I think its fine for the tabs to exist in the test expectations (even if they are ignored). Makes it easier to copy and paste output into expectations.
Yes. And that's the reason I wanted to here people's opinions. The reason I wanted to remove them was we were not consistent on that; most of out CHECK lines don't have tabs, only some of them do. So I thought like, if we are not gonna make very CHECK line have tabs, maybe remove them all is consistent. But anyway, as you said, these are generated test lines after all and I don't feel strongly about removing them. I was mostly bored last night. :$
I'm tempted to just say leave these as is. Unless we plan to enforce a policy here, which I'm not convinced we should.
It would be awesome if we could somehow force tabs to be errors, frankly. Or is there some other automatic way to detect them? Or show them in code review?
I know this changes is abandoned, but I am in favor of it.
+1. Perhaps we could all agree on a stand git pre-commit hook. I use one that doesn't let me commit unless I've run git clang-format, and I'm sure it could be extended to check for the absence of tabs as well.
But what about then the tabs form part of the program under test's output? Whats that argument against allowing them? Surely it makes it easier if one can (or at least has the option to) simply copy and paste the expectations?
Yeah, that's fair. And I've never seen an issue where a tab appeared in a source file. I have had issues with white space at the ends of lines in tests, though. My editor likes to automatically remove them whenever I save those files, which causes extra noise in my diffs that I have to manually undo. So a policy against end-of-line white space would actually be helpful for me.