Skip to content

Ignore exit code of clang-format-diff #35

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Ignore exit code of clang-format-diff #35

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c23a10a
Fix typo douple -> double
ptomato Jul 28, 2024
b90649d
tests: Print output when git command fails
ptomato Jul 28, 2024
8c6e901
tests: Allow file protocol when creating git submodules for tests
ptomato Jul 28, 2024
e1dbe13
Ignore exit code of clang-format-diff
ptomato Jul 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions apply-format
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,8 +325,7 @@ else # Diff-only.
-p1 \
-style="$style" \
-iregex="$exclusions_regex"'.*\.(c|cpp|cxx|cc|h|hpp|m|mm|js|java)' \
> "$patch_dest" \
|| exit 1
> "$patch_dest"
Copy link
Contributor

@gareth-rees gareth-rees Mar 10, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it makes sense to ignore all exit codes. From version 18, clang-format-diff exits with status 1 if there are diffs, but other statuses are possible, for example, if there is an invalid command-line option then the exit status is 2:

$ /usr/share/clang/clang-format-18/clang-format-diff.py --no-such-option
usage: clang-format-diff.py [-h] [-i] [-p NUM] [-regex PATTERN] [-iregex PATTERN] [-sort-includes]
                            [-v] [-style STYLE] [-fallback-style FALLBACK_STYLE] [-binary BINARY]
clang-format-diff.py: error: unrecognized arguments: --no-such-option
$ echo $?
2

So we ought to follow the clang-format-diff pipeline with something like:

[ $? -gt 1 ] && exit $?


if [ "$apply_to_staged" = true ]; then
if [ ! -s "$patch_dest" ]; then
Expand Down
6 changes: 5 additions & 1 deletion tests/mixin_git.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,11 @@ def check_output(self, *args, **kwargs):
kwargs['stderr'] = subprocess.STDOUT
kwargs['universal_newlines'] = True
with self.work_dir():
return subprocess.check_output(args, **kwargs)
try:
return subprocess.check_output(args, **kwargs)
except subprocess.CalledProcessError as exc:
exc.add_note(exc.output)
raise exc
Comment on lines -71 to +75
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change seems to be unrelated to the clang-format-diff exit status. I suggest making separate PR(s) for this and the other changes.


def git_check_call(self, *args):
return self.check_call('git', *args)
Expand Down
6 changes: 4 additions & 2 deletions tests/mixin_scripts_repo.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,8 @@ def setUp(self):
self.repo = GitRepository(worktree_branch_path)

# The new module may have submodules, make sure they are synced.
self.repo.git_check_output('submodule', 'update', '--init', '--recursive')
self.repo.git_check_output('-c', 'protocol.file.allow=always',
'submodule', 'update', '--init', '--recursive')

# If case we cloned the current repo but there's unstaged content.
self.update_scripts()
Expand Down Expand Up @@ -169,7 +170,8 @@ def config_repo(self):

def new_repo_with_submodule(self):
repo = self.new_repo()
repo.git_check_output('submodule', 'add', self.this_repo_path(), self.SUBMODULE_DIR)
repo.git_check_output('-c', 'protocol.file.allow=always', 'submodule', 'add',
self.this_repo_path(), self.SUBMODULE_DIR)
repo.commit()
return repo

Expand Down
4 changes: 2 additions & 2 deletions tests/test_apply_format.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,8 @@ def test_two_files(self):
# Two files need changes.
self.repo.write_file(data.FILENAME_ALT, data.CODE)
output = self.apply_format_output()
douple_patch = data.PATCH + data.PATCH.replace(data.FILENAME, data.FILENAME_ALT)
self.assertEqual(self.simplify_diff(output), douple_patch)
double_patch = data.PATCH + data.PATCH.replace(data.FILENAME, data.FILENAME_ALT)
self.assertEqual(self.simplify_diff(output), double_patch)

# Stage the second file. Two need changes, one is staged the other isn't.
self.repo.add(data.FILENAME_ALT)
Expand Down