The reason to make it a shell script it's that it needs almost no logic, it's just a shortcut for a couple of commands. The hard part here was incompatibility of sed on different platforms. If this script was written in Python, there would also be a chance of hitting compatibility issues between Python versions (as it was with tests for clang-tidy-diff.py, it uses argparse which is available only in python 2.7+).
But if someone can suggest a significantly better alternative to this script, which is compatible with all configurations supported by llvm, I won't object. On 8 Aug 2014 20:00, "Rui Ueyama" <[email protected]> wrote: > I don't see any reason why it's a shell script and not written in Python. > After the 5 commits to try to fix sed invocation Python seems obviously > better than the shell. > > > On Fri, Aug 8, 2014 at 9:44 AM, Nico Weber <[email protected]> wrote: > >> On Fri, Aug 8, 2014 at 2:00 AM, Alexander Kornienko <[email protected]> >> wrote: >> >>> On Fri, Aug 8, 2014 at 10:34 AM, Alexander Kornienko <[email protected]> >>> wrote: >>> >>>> On Thu, Aug 7, 2014 at 11:49 PM, Benjamin Kramer < >>>> [email protected]> wrote: >>>> >>>>> ... >>>>> >>>>> --- clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh >>>>> (original) >>>>> +++ clang-tools-extra/trunk/test/clang-tidy/check_clang_tidy_fix.sh >>>>> Thu Aug 7 16:49:38 2014 >>>>> @@ -20,7 +20,7 @@ set -o errexit >>>>> # Remove the contents of the CHECK lines to avoid CHECKs matching on >>>>> themselves. >>>>> # We need to keep the comments to preserve line numbers while >>>>> avoiding empty >>>>> # lines which could potentially trigger formatting-related checks. >>>>> -sed 's#// *[A-Z-]\+:.*#//#' ${INPUT_FILE} > ${TEMPORARY_FILE} >>>>> +sed -E 's#// *[A-Z-]+:.*#//#' ${INPUT_FILE} > ${TEMPORARY_FILE} >>>>> >>>> >>>> The check is awesome, but the idea to improve sed invocation turned out >>>> to be not very nice: r215163, r215171, r215175, and finally back to what it >>>> was: r215189. Let's leave it as is ;) >>>> >>> >>> I was also wrong trying to change sed invocation after Rui's last >>> attempt, which actually worked both on Linux and Darwin, hence r215192. >>> >>> Compatibility is hard =/ >>> >> >> Any reason this is a shell script, not a python script? >> >> >>> >>> _______________________________________________ >>> cfe-commits mailing list >>> [email protected] >>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >>> >>> >> >> _______________________________________________ >> cfe-commits mailing list >> [email protected] >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >> >> >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
