Merged.

BTW: I have set up my editor to run uncrustify on every file change and I'm not looking back :-)

Also fine with me.

Fine with me. Like Malte, I'll miss the diff option and the option to run only on a specific file but I can live without it.

1) I agree that a "force" option could be useful. I added it to the patch.

2) Using the run-uncrustify.py script directly produces less output.

Can I merge this?

I'm done with my review and only had a short comment on the code.

I also tried the process on git and it worked there. There are two things that bug me a bit, but I could live with both of them:


1) When I have uncommitted changes, I can run the style checks to see that there are problems but to see what the problems actually are, I have to commit the faulty version first. It seems like a strange incentive for people to check in wrongly formatted code. A "diff" option would be nice here, and maybe also a "force" option to do the modifications even if the changes are not committed.

2) The actual problem is a bit hidden in the output if there are style issues.
For example, this is the output if there is a problem with just one file:

========================================
style installed: appdirs==1.4.3,CacheControl==0.12.6,certifi==2019.11.28,chardet==3.0.4,colorama==0.4.3,contextlib2==0.6.0,distlib==0.3.0,distro==1.4.0,flake8==3.8.3,html5lib==1.0.1,idna==2.8,ipaddr==2.2.0,lockfile==0.12.2,mccabe==0.6.1,msgpack==0.6.2,packaging==20.3,pep517==0.8.2,progress==1.5,pycodestyle==2.6.0,pyflakes==2.2.0,pyparsing==2.4.6,pytoml==0.1.21,requests==2.22.0,retrying==1.3.3,six==1.14.0,urllib3==1.25.8,webencodings==0.5.1
style run-test-pre: PYTHONHASHSEED='1660022466'
style run-test: commands[0] | python run-all-style-checks.py
Running check_cc_files
Checking style of 210 *.cc files
Running check_cplusplus_style
Checking 432 files with uncrustify.
FAIL: /home/pommeren/Desktop/convert/converted/jendrik-downward/src/search/command_line.h (File size changed from 559 to 557)
Run "tox -e fix-style" in the misc/ directory to fix the C++ style.
Running check_include_guard_convention
Running check_python_style
Style checks failed
ERROR: InvocationError for command /home/pommeren/Desktop/convert/converted/jendrik-downward/misc/.tox/style/bin/python run-all-style-checks.py (exited with code 1)
_____________________________________________________________________________________________________________________________ summary ______________________________________________________________________________________________________________________________
ERROR:   style: commands failed
========================================

The last two ERROR lines are red so they draw focus but the actual important information is in line 9/16 which is not red. Likewise, if there are uncommitted changes, I get 9 lines of output (two of them in red) but the actual information is somewhere in the middle.

I had a quick look, not a real review, but from my side I don't need to review this again. If it works, it works.

New pull request: https://github.com/aibasel/jendrik-downward/pull/1

Florian, if the bitbucket repo is gone before you get a chance to look at the patch, you can find the code at ssh://ai-repos/seipp/projects/downward .

Thanks for the tips, Malte! I updated the patch accordingly.

Florian, are you also happy with the patch?

I'm not sure I understand what exactly you mean, but I had a look and left some comments. The proof is in the eating, though. If it works for git and hg for you and Florian, fine to merge.

I liked the --diff option and used it frequently, but I'll survive if it's gone.

I made a pull request at https://bitbucket.org/jendrikseipp/downward/pull-requests/154 

It would be great if we could merge this today, so I don't have to "redo" everything in git. The new script allows for checking and modifying files but doesn't provide a diff of changes. Given that the script will only modify a clean repo, I don't think a diff option is necessary.

Malte, are you fine with the patch?

Notes copied from the task board:

Maybe have a script (maybe part of a new tox -e fix-style) with options to check, show diff without changes, and modify (refuse to modify anything with there are uncommitted changes).

The buildbot workers still install mercurial and set up "hg uncrustify". Once we have an alternative (and they don’t have to pull from hg.fast-downward.org anymore), we should update them as well.

We want to find a replacement for "hg uncrustify -m".