Skip to content

Simplify #36

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

Merged
mernst merged 2 commits into from
Apr 7, 2026
Merged

Simplify #36

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
92be208
Simplify
mernst Apr 7, 2026
b804600
Undo changes
mernst Apr 7, 2026
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: 2 additions & 1 deletion git-clone-related
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@
# Do not attempt to add "--filter=blob:none" to the git command, as older git
# clients (apparently including GitHub, as of 2024-01-27) do not support it.

# Fail if any command fails.
# Do not fail on error; this script handles errors explicitly.
set +e
Comment on lines +43 to 44
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Apr 7, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Expect both commands to reach the final marker, demonstrating that `set +e`
# no longer aborts on the patterns this script currently relies on.
/bin/sh -c 'set +e; if ! false; then false; fi; echo survived_after_false'
/bin/sh -c 'set +e; value="$(false)"; status=$?; printf "status=%s value=<%s>\n" "$status" "$value"; echo survived_after_substitution'

Repository: plume-lib/git-scripts

Length of output: 131

🏁 Script executed:

# First, locate and examine the git-clone-related file
fd 'git-clone-related$' -type f

Repository: plume-lib/git-scripts

Length of output: 235

🏁 Script executed:

# Check if git-clone-related exists and examine its structure
git ls-files | grep -E 'git-clone-related|git-find-fork|git-find-branch'

Repository: plume-lib/git-scripts

Length of output: 205

🏁 Script executed:

wc -l git-clone-related git-find-fork git-find-branch

Repository: plume-lib/git-scripts

Length of output: 138

🏁 Script executed:

cat -n git-clone-related | head -50

Repository: plume-lib/git-scripts

Length of output: 2828

🏁 Script executed:

cat -n git-clone-related | sed -n '100,190p'

Repository: plume-lib/git-scripts

Length of output: 4799

🏁 Script executed:

cat -n git-clone-related | tail -20

Repository: plume-lib/git-scripts

Length of output: 1223

🏁 Script executed:

cat -n git-find-fork

Repository: plume-lib/git-scripts

Length of output: 1597

🏁 Script executed:

cat -n git-find-branch

Repository: plume-lib/git-scripts

Length of output: 742

🏁 Script executed:

cat -n tests/git-clone-related-test/test-git-clone-related.sh | sed -n '1,50p'

Repository: plume-lib/git-scripts

Length of output: 2094

🏁 Script executed:

# Check what happens after lines 165, 172, 182 - are there any error checks?
cat -n git-clone-related | sed -n '163,175p'

Repository: plume-lib/git-scripts

Length of output: 894

🏁 Script executed:

cat -n git-clone-related | sed -n '180,186p'

Repository: plume-lib/git-scripts

Length of output: 567

🏁 Script executed:

# Verify git-find-branch error behavior more clearly
cat -n git-find-branch

Repository: plume-lib/git-scripts

Length of output: 742

🏁 Script executed:

# Check what happens after the false statement at line 153 - does script continue or exit?
cat -n git-clone-related | sed -n '145,160p'

Repository: plume-lib/git-scripts

Length of output: 653

🏁 Script executed:

# Verify the actual behavior: with set +e, does false cause exit or just set status?
/bin/sh -c 'set +e; if ! false; then echo "in if block"; false; fi; echo "after if"'

Repository: plume-lib/git-scripts

Length of output: 85

set +e disables fail-fast error handling that the script relies on.

The script uses set +e but several code patterns assume execution will stop on command failure:

  • Line 153 false does not halt the script; execution continues to line 156+
  • Lines 165, 172, 182 capture command output without checking exit codes; if git-find-fork or git-find-branch fail, the variables receive empty or partial output
  • Line 147–154 checks eval "${ci_info_output}" but the false on line 153 no longer terminates execution when eval fails
  • The test file (tests/git-clone-related-test/test-git-clone-related.sh:21) uses set -o errexit, expecting the script to honor fail-fast semantics

This delays error detection until the git clone command at line 189 fails with unset or incorrect parameters (empty REPO_BRANCH or REPO_URL).

Either restore set -e or wrap lines 137, 165, 172, and 182 with explicit error checks (e.g., if ! ... ; then exit 1; fi).

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@git-clone-related` around lines 43 - 44, The script currently disables
fail-fast with "set +e", causing operations like eval "${ci_info_output}", the
false sentinel, and calls to git-find-fork / git-find-branch to continue on
failure and produce empty REPO_BRANCH/REPO_URL; either restore "set -e" at the
top to re-enable errexit or add explicit checks around the failing-sensitive
commands (wrap eval "${ci_info_output}", the `false` sentinel, and the
git-find-fork / git-find-branch invocations with if ! ...; then exit 1; fi style
checks) so failures are detected immediately and REPO_BRANCH/REPO_URL are never
left unset.


if [ "$1" = "--debug" ]; then
Expand Down Expand Up @@ -111,6 +111,7 @@ else
for dir in "../plume-scripts" ".plume-scripts" "../.plume-scripts"; do
if [ -d "${SCRIPT_DIR}/${dir}" ]; then
PLUME_SCRIPTS="${SCRIPT_DIR}/${dir}"
break
fi
done
if [ -z "$PLUME_SCRIPTS" ]; then
Expand Down
6 changes: 3 additions & 3 deletions resolve-adjacent-conflicts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
#!/bin/bash
# bash, not POSIX sh, because of "readarray".

echo "Please use the program in https://github.com/plume-lib/merging ."
echo "You are using $0,"
echo "which is an obsolete version in https://github.com/plume-lib/git-scripts ."
echo "Please use the program in https://github.com/plume-lib/merging ." >&2
echo "You are using $0," >&2
echo "which is an obsolete version in https://github.com/plume-lib/git-scripts ." >&2

# This script edits files in place to resolve conflict markers due to edits on
# adjacent lines. (This is like the behavior of SVN and darcs, but different
Expand Down
20 changes: 3 additions & 17 deletions resolve-conflicts.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@

from __future__ import annotations

import itertools
import shutil
import sys
import tempfile
Expand Down Expand Up @@ -64,13 +63,7 @@ def main() -> None: # pylint: disable=too-many-locals
args = arg_parser.parse_args()
filename = args.filename

num_options = 0
if args.adjacent_lines:
num_options += 1
if args.blank_lines:
num_options += 1
if args.java_imports:
num_options += 1
num_options = sum([args.adjacent_lines, args.blank_lines, args.java_imports])
if num_options != 1:
print("resolve-conflicts.py: supply exactly one option.")
sys.exit(1)
Expand Down Expand Up @@ -268,11 +261,7 @@ def merge_edits_on_different_lines(
result: list[str] | None = None
if base_len == len(parent1) and base_len == len(parent2):
result = []
for base_line, parent1_line, parent2_line in itertools.zip_longest(
base,
parent1,
parent2,
):
for base_line, parent1_line, parent2_line in zip(base, parent1, parent2, strict=True):
Comment thread
mernst marked this conversation as resolved.
debug_print("Considering line:", base_line, parent1_line, parent2_line)
if parent1_line == parent2_line:
result.append(parent1_line)
Expand Down Expand Up @@ -395,10 +384,7 @@ def with_one_space(lines: list[str]) -> str:
# TODO: This could be more efficient. Even better, I could write a loop in
# merge_blank_lines that wouldn't need to create new strings at all. But this is
# expedient to write and is probably fast enough.
result_lines = []
for line in lines:
result_lines += line.split()
return " ".join(result_lines)
return " ".join(word for line in lines for word in line.split())


def debug_print(*args: object) -> None:
Expand Down
6 changes: 3 additions & 3 deletions resolve-import-conflicts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
#!/bin/bash
# bash, not POSIX sh, because of "readarray".

echo "Please use the program in https://github.com/plume-lib/merging ."
echo "You are using $0,"
echo "which is an obsolete version in https://github.com/plume-lib/git-scripts ."
echo "Please use the program in https://github.com/plume-lib/merging ." >&2
echo "You are using $0," >&2
echo "which is an obsolete version in https://github.com/plume-lib/git-scripts ." >&2

# This script edits files in place to remove conflict markers related to Java
# imports. For a given conflict that involves only `import` statements and
Expand Down