Repository navigation
Conversation
47b0959 to
35e7bf2
Compare
| item.unlink() | ||
|
|
||
|
|
||
| def diff_has_single_added_block(diff_cmd: list[str]) -> bool: |
There was a problem hiding this comment.
| def diff_has_single_added_block(diff_cmd: list[str]) -> bool: | |
| def diff_has_single_added_block(diff_cmd: list[str]) -> tuple[bool, str]: |
| # Ignore diff header lines (e.g., '+++ os-autoinst.changes...') | ||
| if line.startswith("+++") or line.startswith("---"): | ||
| continue | ||
|
|
||
| if line.startswith("-"): | ||
| return (False, None) | ||
|
|
||
| if line.startswith("+"): | ||
| if not in_added_block: | ||
| in_added_block = True | ||
| block_count += 1 | ||
| else: | ||
| in_added_block = False | ||
|
|
There was a problem hiding this comment.
this should be equivalent
| # Ignore diff header lines (e.g., '+++ os-autoinst.changes...') | |
| if line.startswith("+++") or line.startswith("---"): | |
| continue | |
| if line.startswith("-"): | |
| return (False, None) | |
| if line.startswith("+"): | |
| if not in_added_block: | |
| in_added_block = True | |
| block_count += 1 | |
| else: | |
| in_added_block = False | |
| lines = [l for l in res.stdout.splitlines() if not l.startswith(("+++", "---"))] | |
| if any(l.startswith("-") for l in lines): | |
| return (False, res.stdout) | |
| blocks = sum(1 for is_add, _ in groupby(lines, key=lambda l: l.startswith("+")) if is_add) |
There was a problem hiding this comment.
I find some of the python stuff very verbose, but in this case I find my version easier to understand
There was a problem hiding this comment.
fine, you can come up with a different way but needing to read a for loop and how a counter variable is tracked just to understand that what you want is a sum can be improved.
Drop the header lines by skipping everything before the first @@. Then use groupby on the first character of each line, so there are no counters and no header filtering:
from itertools import dropwhile, groupby
def diff_has_single_added_block(diff_cmd: list[str]) -> tuple[bool, subprocess.CompletedProcess[str]]:
"""Return True if the diff only adds one contiguous block of lines."""
res = _run_subprocess(diff_cmd)
hunk_lines = dropwhile(lambda line: not line.startswith("@@"), res.stdout.splitlines())
runs = [kind for kind, _ in groupby(line[:1] for line in hunk_lines)]
return ("-" not in runs and runs.count("+") == 1, res)Returning the CompletedProcess keeps the callers' res.stdout working and removes the None path
Also uses dropwhile which I find quite descriptive about what it does. Sure, the first time you see it, you might be surprised but the intention should be very clear just from the function name. Also here reading a for loop and understanding the effect of "continue" might use simple control statements but one still needs to read the whole flow to understand what is going on. dropwhile and groupby in their lines when used make it clear how those lines are processed in the consecutive statements.
3d3fd12 to
5d3b5ab
Compare
Issue: https://progress.opensuse.org/issues/205521