Performace/profiler reports for the indentation of the Seed7 files in lib and prg directories#140
Open
pierre-rouleau wants to merge 12 commits into
Open
Performace/profiler reports for the indentation of the Seed7 files in lib and prg directories#140pierre-rouleau wants to merge 12 commits into
pierre-rouleau wants to merge 12 commits into
Conversation
…prg files. The prg files are included up to chkscan.sd7
|
Important Review skippedToo many files! This PR contains 3000 files, which is 2900 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3000)
You can disable this status message by setting the ✨ Finishing Touches🧪 Generate unit tests (beta)
|
Owner
Author
|
@coderabbitai Please analyze the performance/profiler reports included in the directories 24.0, 25.0 and 26.0 and see if there are regressions in the performances that could be explained and where code could be improved. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
# Get PR files list to locate the directories 24.0, 25.0, 26.0
gh pr view 140 --repo pierre-rouleau/seed7-mode --json files --jq '.files[].path' | head -100Length of output: 3670 🏁 Script executed: #!/bin/bash
set -euo pipefail
gh pr view 140 --repo pierre-rouleau/seed7-mode --json files --jq '.files[].path' > /tmp/allfiles.txt
wc -l /tmp/allfiles.txt
for v in 24.0 25.0 26.0; do
echo "=== $v ==="
grep "^reports/$v/" /tmp/allfiles.txt | grep -c 'perf-cpu.txt' || true
grep "^reports/$v/" /tmp/allfiles.txt | grep -c 'perf-mem.txt' || true
grep "^reports/$v/timing" /tmp/allfiles.txt || true
doneLength of output: 622 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "--- ls reports ---"
ls reports/ 2>/dev/null || fd -t d 'reports' -d 2
echo "--- 24.0 timing files ---"
fd -g 'timing*.txt' reports/24.0 2>/dev/null
echo "--- 25.0 timing files ---"
fd -g 'timing*.txt' reports/25.0 2>/dev/null
echo "--- 26.0 timing files ---"
fd -g 'timing*.txt' reports/26.0 2>/dev/null
echo "--- counts ---"
for v in 24.0 25.0 26.0; do
echo "=== $v ==="
find reports/$v -maxdepth 1 -name '*-perf-cpu.txt' 2>/dev/null | wc -l
find reports/$v -maxdepth 1 -name '*-perf-mem.txt' 2>/dev/null | wc -l
find reports/$v -maxdepth 1 -iname 'timing*' 2>/dev/null
doneLength of output: 2324 🏁 Script executed: #!/bin/bash
set -euo pipefail
for v in 24.0 25.0 26.0; do
echo "=== $v header ==="
grep '^#' reports/$v/timing.txt || echo "(no header comments)"
echo "--- total lines ---"
wc -l reports/$v/timing.txt
echo "--- first 10 data lines ---"
grep -v '^#' reports/$v/timing.txt | head -10
doneLength of output: 4223 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 << 'EOF'
import re
def parse(path):
data = {}
with open(path) as f:
for line in f:
if line.startswith('#') or not line.strip():
continue
m = re.match(r'(\S+)\s*:\s*indented in:\s*([\d.]+)\s*seconds\.\s*(\d+)\s*lines', line)
if m:
name, t, lines = m.groups()
data[name] = (float(t), int(lines))
return data
v24 = parse('reports/24.0/timing.txt')
v25 = parse('reports/25.0/timing.txt')
v26 = parse('reports/26.0/timing.txt')
print("counts:", len(v24), len(v25), len(v26))
common_25_26 = set(v25) & set(v26)
print("\n=== Top regressions 25.0 -> 26.0 (by absolute time increase) ===")
rows = []
for name in common_25_26:
t25, l25 = v25[name]
t26, l26 = v26[name]
if l25 == l26 and t25 > 0:
ratio = t26 / t25
rows.append((t26 - t25, ratio, name, t25, t26, l25))
rows.sort(reverse=True)
for diff, ratio, name, t25, t26, lines in rows[:20]:
print(f"{name:20s} lines={lines:5d} t25={t25:8.3f}s t26={t26:8.3f}s ratio={ratio:6.2f}x diff={diff:8.3f}s")
print("\n=== Top regressions by ratio (25.0 -> 26.0), min 0.5s baseline ===")
rows2 = [r for r in rows if r[3] > 0.5]
rows2.sort(key=lambda r: r[1], reverse=True)
for diff, ratio, name, t25, t26, lines in rows2[:20]:
print(f"{name:20s} lines={lines:5d} t25={t25:8.3f}s t26={t26:8.3f}s ratio={ratio:6.2f}x diff={diff:8.3f}s")
common_24_25 = set(v24) & set(v25)
print("\n=== Top regressions 24.0 -> 25.0 ===")
rows3 = []
for name in common_24_25:
t24, l24 = v24[name]
t25, l25 = v25[name]
if l24 == l25 and t24 > 0:
ratio = t25 / t24
rows3.append((t25 - t24, ratio, name, t24, t25, l24))
rows3.sort(reverse=True)
for diff, ratio, name, t24, t25, lines in rows3[:20]:
print(f"{name:20s} lines={lines:5d} t24={t24:8.3f}s t25={t25:8.3f}s ratio={ratio:6.2f}x diff={diff:8.3f}s")
# check x509cert specifically
for label, d in [('24.0', v24), ('25.0', v25), ('26.0', v26)]:
if 'x509cert.s7i' in d:
print(f"x509cert.s7i in {label}: {d['x509cert.s7i']}")
EOFLength of output: 8215 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== aes-perf-cpu.txt 25.0 head ==="
head -60 reports/25.0/aes-perf-cpu.txt
echo ""
echo "=== aes-perf-cpu.txt 26.0 head ==="
head -60 reports/26.0/aes-perf-cpu.txtLength of output: 7699 🏁 Script executed: #!/bin/bash
set -euo pipefail
fd -e el . -x echo {} 2>/dev/null | grep -i seed7-mode || fd '.el$'
echo "--- outline of seed7-mode.el functions of interest ---"
FILE=$(fd -t f 'seed7-mode.el$' | head -1)
echo "FILE=$FILE"
grep -n "defun seed7-line-inside-parens-pair" -A 60 "$FILE" | head -120Length of output: 7131 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE=seed7-mode.el
echo "=== seed7-re-search-backward ==="
grep -n "defun seed7-re-search-backward" -A 40 "$FILE" | head -60
echo "=== seed7-inside-comment-p ==="
grep -n "defun seed7-inside-comment-p" -A 30 "$FILE" | head -40
echo "=== seed7--open-paren-regexp ==="
grep -n "seed7--open-paren-regexp" "$FILE" | head -10
echo "=== recent commits touching seed7-mode.el ==="
git log --oneline -n 30 -- seed7-mode.elLength of output: 7264 shell echo "=== 45ae853 commit message and diff stat ===" echo "=== diff for seed7-mode.el in 45ae853 ===" |
Owner
Author
|
@coderabbitai. Please analyze the timing.txt file of the 3 following directories: 23.0, 24.0 and 25.0 and identify files that would need further investigation that would help speed up processing the indentation. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
for v in 23.0 24.0 25.0; do
echo "=== $v header ==="
grep '^#' reports/$v/timing.txt || echo "(no header comments)"
echo "--- total lines ---"
wc -l reports/$v/timing.txt
doneLength of output: 1365 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 << 'EOF'
import re
def parse(path):
data = {}
with open(path) as f:
for line in f:
if line.startswith('#') or not line.strip():
continue
m = re.match(r'(\S+)\s*:\s*indented in:\s*([\d.]+)\s*seconds\.\s*(\d+)\s*lines', line)
if m:
name, t, lines = m.groups()
data[name] = (float(t), int(lines))
return data
v23 = parse('reports/23.0/timing.txt')
v24 = parse('reports/24.0/timing.txt')
v25 = parse('reports/25.0/timing.txt')
print("counts:", len(v23), len(v24), len(v25))
def compare(a, b, la, lb):
common = set(a) & set(b)
rows = []
for name in common:
ta, na = a[name]
tb, nb = b[name]
if na == nb and ta > 0:
ratio = tb / ta
rows.append((tb - ta, ratio, name, ta, tb, na))
rows.sort(reverse=True)
print(f"\n=== Top regressions {la} -> {lb} (by absolute time increase) ===")
for diff, ratio, name, ta, tb, lines in rows[:15]:
print(f"{name:20s} lines={lines:5d} t_{la}={ta:8.3f}s t_{lb}={tb:8.3f}s ratio={ratio:6.2f}x diff={diff:8.3f}s")
print(f"\n=== Top regressions by ratio {la} -> {lb} (baseline > 0.3s) ===")
rows2 = [r for r in rows if r[3] > 0.3]
rows2.sort(key=lambda r: r[1], reverse=True)
for diff, ratio, name, ta, tb, lines in rows2[:15]:
print(f"{name:20s} lines={lines:5d} t_{la}={ta:8.3f}s t_{lb}={tb:8.3f}s ratio={ratio:6.2f}x diff={diff:8.3f}s")
return rows
r1 = compare(v23, v24, '23.0', '24.0')
r2 = compare(v24, v25, '24.0', '25.0')
print("\n=== Time per line ranking in 25.0 (slowest per-line) ===")
per_line = []
for name, (t, lines) in v25.items():
if lines > 0:
per_line.append((t/lines, t, lines, name))
per_line.sort(reverse=True)
for tpl, t, lines, name in per_line[:20]:
print(f"{name:20s} lines={lines:5d} total={t:8.3f}s per_line={tpl*1000:8.3f}ms")
print("\n=== Files consistently slow across all 3 versions (present everywhere, top 15 by 25.0 time) ===")
common_all = set(v23) & set(v24) & set(v25)
rows3 = sorted(common_all, key=lambda n: -v25[n][0])[:15]
for name in rows3:
print(f"{name:20s} t23={v23[name][0]:8.3f}s t24={v24[name][0]:8.3f}s t25={v25[name][0]:8.3f}s lines={v25[name][1]}")
EOFLength of output: 11383 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 << 'EOF'
import re
def parse(path):
data = {}
with open(path) as f:
for line in f:
if line.startswith('#') or not line.strip():
continue
m = re.match(r'(\S+)\s*:\s*indented in:\s*([\d.]+)\s*seconds\.\s*(\d+)\s*lines', line)
if m:
name, t, lines = m.groups()
data[name] = (float(t), int(lines))
return data
v23 = parse('reports/23.0/timing.txt')
v24 = parse('reports/24.0/timing.txt')
v25 = parse('reports/25.0/timing.txt')
for name in ['chkset.sd7','chkstr.sd7','chkint.sd7','chkchr.sd7','bas7.sd7','elliptic.s7i']:
print(name, 'in23:', v23.get(name), 'in24:', v24.get(name), 'in25:', v25.get(name))
EOF
echo "=== find profiler reports for chkset.sd7 ==="
fd 'chkset' reports/25.0
fd 'chkstr' reports/25.0
echo "=== check if these files are new-only files ==="
ls reports/25.0 | grep -E 'chkset|chkstr'Length of output: 1826 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== chkset-perf-cpu.txt head ==="
head -80 reports/25.0/chkset-perf-cpu.txt
echo ""
echo "=== chkstr-perf-cpu.txt head ==="
head -80 reports/25.0/chkstr-perf-cpu.txtLength of output: 10432 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE=seed7-mode.el
echo "=== seed7-re-search-backward full definition ==="
grep -n "defun seed7-re-search-backward\b" -A 60 "$FILE"
echo ""
echo "=== seed7-re-search-backward-closest ==="
grep -n "defun seed7-re-search-backward-closest" -A 40 "$FILE"
echo ""
echo "=== seed7-line-is-defun-end ==="
grep -n "defun seed7-line-is-defun-end" -A 40 "$FILE"
echo ""
echo "=== seed7-beg-of-defun ==="
grep -n "defun seed7-beg-of-defun" -A 40 "$FILE"
echo ""
echo "=== seed7-line-inside-parens-pair (not column) ==="
grep -n "defun seed7-line-inside-parens-pair\b" -A 40 "$FILE"Length of output: 17397
1. Minor, explainable regressions (23.0 → 24.0, 24.0 → 25.0)Most files show small regressions (1–13%), correlating with the noted feature changes:
2. The real targets:
|
| File | Lines | Total time | ms/line |
|---|---|---|---|
chkset.sd7 |
11,973 | 18,844s | 1573.9 |
chkstr.sd7 |
26,951 | 7,592s | 281.7 |
chkint.sd7 (comparable size) |
38,128 | 4,598s | 120.6 |
bas7.sd7 (comparable size) |
11,458 | 1,600s | 139.7 |
chkset.sd7 is ~13x slower per line than a similarly-structured file (chkint.sd7), and chkstr.sd7 is ~2x slower than bas7.sd7. These point to two distinct, fixable hot spots:
a) Timer creation overhead in seed7-re-search-backward (line 3010)
In chkstr-perf-cpu.txt, 62% of samples are under seed7-line-inside-parens-pair-column → seed7-re-search-backward, and within that, 38% of total profiled time is spent in run-with-timer → run-at-time → timer-activate → timer--time-less-p — i.e., Emacs's real timer machinery, not the regexp search itself.
The cause: seed7-re-search-backward wraps its entire search loop in (with-timeout (10 ...) ...), which creates and cancels a genuine Emacs timer on every call. seed7-line-inside-parens-pair (line 6923) calls seed7-re-search-backward in a loop — once per enclosing paren level, for every line. For files with deeply/frequently nested parens (array/record literals, as in chkstr.sd7), this means creating and tearing down a timer many times per line, and the timer bookkeeping (not the search) becomes the dominant cost.
Suggested fix: don't create a timer per search call. Options:
- Move the 10s timeout to a single outer
with-timeoutwrapping the wholeseed7-indent-region/seed7-calc-indentcall (one timer per top-level indent operation instead of one per backward search), or - Replace the timer-based timeout inside
seed7-re-search-backwardwith a lightweight manual deadline check (float-timecomparison checked only every N loop iterations) instead ofwith-timeout.
b) Unbounded backward scan in seed7-beg-of-defun triggered per line (line 3608)
In chkset-perf-cpu.txt, 79% of samples are under seed7-line-is-defun-end → seed7-beg-of-defun → seed7-re-search-backward-closest → seed7-re-search-backward → re-search-backward. seed7-beg-of-defun performs an unbounded backward scan to locate the enclosing function/procedure declaration, and it appears to be invoked very frequently — likely because chkset.sd7 contains many case/select/block-end lines, each passing the "end"/"const" fast-reject check in seed7-line-is-defun-end and triggering a full backward scan. Repeated O(n) backward scans across many lines in an 11,973-line file produce near-quadratic behavior.
Suggested fix: bound the backward search in seed7-beg-of-defun/seed7-re-search-backward-closest to a nearer known boundary (e.g., the previous processed defun boundary or a cached position) instead of scanning unboundedly toward (point-min) on every invocation, or memoize/cache defun-start lookups within a single seed7-indent-region pass.
Recommendation
Prioritize (a) and (b) above — they're the clearest, most actionable regressions since they represent genuine algorithmic/timer-overhead issues rather than incidental cost of correctness fixes. Want me to open a follow-up issue for the seed7-re-search-backward timer overhead and the unbounded seed7-beg-of-defun scan?
Owner
Author
|
@coderabbitai Please propose code to implement the recommendation a and b, based on the latest version of seed7-mode. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE=seed7-mode.el
echo "=== seed7-re-search-backward ==="
grep -n "defun seed7-re-search-backward\b" -A 60 "$FILE"
echo "=== seed7-re-search-backward-closest ==="
grep -n "defun seed7-re-search-backward-closest\b" -A 50 "$FILE"
echo "=== seed7-line-inside-parens-pair-column ==="
grep -n "defun seed7-line-inside-parens-pair-column\b" -A 60 "$FILE"
echo "=== seed7-line-inside-parens-pair\b (exact) ==="
grep -n "defun seed7-line-inside-parens-pair\b" -A 60 "$FILE"
echo "=== seed7-line-is-defun-end ==="
grep -n "defun seed7-line-is-defun-end\b" -A 40 "$FILE"
echo "=== seed7-beg-of-defun ==="
grep -n "defun seed7-beg-of-defun\b" -A 60 "$FILE"
echo "=== seed7-calc-indent / seed7-indent-region (entry points for outer timeout) ==="
grep -n "defun seed7-calc-indent\b\|defun seed7-indent-region\b\|defun seed7-indent-line\b" -A 20 "$FILE"
echo "=== with-timeout usages ==="
grep -n "with-timeout" "$FILE"Length of output: 28114 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE=seed7-mode.el
echo "=== second with-timeout (line 3375) context ==="
sed -n '3350,3420p' "$FILE"
echo "=== callers of seed7-beg-of-defun ==="
grep -n "seed7-beg-of-defun" "$FILE"
echo "=== callers of seed7-line-is-defun-end ==="
grep -n "seed7-line-is-defun-end" "$FILE"
echo "=== seed7--indent-search-boundary-cache / uncached fn ==="
grep -n "seed7--indent-search-boundary-cache\|seed7--indent-search-boundary-uncached" "$FILE" | head -40
echo "=== rest of seed7-line-is-defun-end (after fast reject) ==="
sed -n '7196,7280p' "$FILE"Length of output: 9755 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE=seed7-mode.el
echo "=== seed7--indent-one-line ==="
grep -n "defun seed7--indent-one-line\b" -A 30 "$FILE"
echo "=== callers of seed7--to-top ==="
grep -n "seed7--to-top" "$FILE"
echo "=== defcustom for timeout duration, if any ==="
grep -n "defcustom.*timeout\|defvar.*timeout" "$FILE"Length of output: 2545
(a) is still live:
After this change, re-run the CPU profiler on ✏️ Learnings added
The requested changes have been implemented and a pull request has been created: View PR |
Owner
Author
|
@coderabbitai I found another indentation error. It is in the lib/asn1.s7i. Please analyze the latest version of seed7-mode and identify the root cause of this indentation bug. Please also propose code to fix this error. Show the proposed code here (not in a separate PR). |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd -t f 'asn1.s7i$'
echo "---"
sed -n '1,110p' lib/asn1.s7i 2>/dev/null || fd asn1.s7i -x sed -n '1,110p' {}Length of output: 281 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "--- find any asn1 file ---"
fd -i asn1
echo "--- list repo root ---"
ls
echo "--- find lib dir ---"
fd -t d '^lib$'Length of output: 1103 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE=seed7-mode.el
echo "=== seed7-calc-indent ==="
grep -n "defun seed7-calc-indent\b" -A 120 "$FILE" | head -160
echo "=== seed7--indent-one-line ==="
grep -n "defun seed7--indent-one-line\b" -A 60 "$FILE"Length of output: 9780 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE=seed7-mode.el
echo "=== seed7-line-inside-array-definition-block ==="
grep -n "defun seed7-line-inside-array-definition-block\b" -A 80 "$FILE"
echo "=== seed7-line-inside-set-definition-block ==="
grep -n "defun seed7-line-inside-set-definition-block\b" -A 40 "$FILE"Length of output: 6942 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE=seed7-mode.el
sed -n '7603,7829p' "$FILE"Length of output: 11530 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE=seed7-mode.el
sed -n '6647,6689p' "$FILE"
echo "=== seed7-move-to-line ==="
grep -n "defun seed7-move-to-line\b" -A 50 "$FILE"Length of output: 4965 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE=seed7-mode.el
grep -n "defun seed7-to-previous-non-empty-line\b" -A 60 "$FILE"Length of output: 3162 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE=seed7-mode.el
grep -n "defun seed7-line-at-endof-set-definition-block\b" -A 45 "$FILE"
echo "=== seed7-line-code-ends-with ==="
grep -n "defun seed7-line-code-ends-with\b" -A 25 "$FILE"Length of output: 4006 Root causeThe bug is in Walking through indentation of line 71 in your example:
So the bug only manifests when the array/set literal's closing line has no code before the closing delimiter (i.e. Proposed fixRelax the middle relation from strict
(seed7--with-backward-sexp
(seed7-to-indent)
(when (looking-at-p seed7--array-definition-start-regexp)
(setq block-start-pos (point))
- (when (< block-start-pos line-start-pos enclosing-block-end-pos line-end-pos)
+ (when (and (< block-start-pos line-start-pos)
+ (<= line-start-pos enclosing-block-end-pos)
+ (< enclosing-block-end-pos line-end-pos))
;; Return the header's own column — the closing line of a
;; nested array literal is aligned with its innermost
;; open paren, not necessarily one `seed7-indent-width'
;; step below the header (see `Te'/`Td' in aes.s7i).
(current-column))))))))))
(seed7--with-backward-sexp
(seed7-to-indent)
(when (looking-at-p seed7--set-definition-start-regexp)
(setq block-start-pos (point))
- (when (< block-start-pos line-start-pos enclosing-block-end-pos line-end-pos)
+ (when (and (< block-start-pos line-start-pos)
+ (<= line-start-pos enclosing-block-end-pos)
+ (< enclosing-block-end-pos line-end-pos))
;; return header's own column
(current-column))))))))))With this change, line 66 ( Would you like me to open a follow-up PR with this fix (and a regression test using this |
Owner
Author
|
@coderabbitai Please propose ERT-based test for testing this type of Seed7 code indentation and detect invalid indentation for it. Please show the proposed code here. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
fd -e el | grep -i test || true
fd -t d test
echo "--- outline of test file if exists ---"
FILE=$(fd -e el | grep -i test | head -1)
echo "FILE=$FILE"
if [ -n "$FILE" ]; then
grep -n "ert-deftest" "$FILE" | head -40
fiLength of output: 1308 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== seed7-test-indent-01.el ==="
cat -n tests/ert-tests/seed7-test-indent-01.el | head -150
echo ""
echo "=== seed7-test-arrays-01.el ==="
cat -n tests/ert-tests/seed7-test-arrays-01.el | head -150
echo ""
echo "=== pel-ert.el helper (used macros) ==="
cat -n tests/ert-tests/pel-ert.el | head -150Length of output: 19535
;;; seed7-test-indent-03.el --- ERT tests for array/set-block-end indentation regressions -*- lexical-binding: t; -*-
;; Author : Pierre Rouleau <prouleau001@gmail.com>
;; This file is part of the SEED7-MODE package.
;; This file is not part of GNU Emacs.
;;; --------------------------------------------------------------------------
;;; Commentary:
;;
;; Regression tests for the case where an array/set-literal definition ends
;; on a line that contains *only* the closing delimiter (e.g. ` );' or
;; ` };'), immediately followed (after blank lines and/or a block comment)
;; by another top-level declaration.
;;
;; Before the fix to `seed7-line-at-endof-array-definition-block' and
;; `seed7-line-at-endof-set-definition-block', the strict chained inequality
;;
;; (< block-start-pos line-start-pos enclosing-block-end-pos line-end-pos)
;;
;; failed whenever `line-start-pos' and `enclosing-block-end-pos' were equal
;; (i.e. the closing delimiter is the very first thing on the line), causing
;; the "just after end of array/set definition block" clause in
;; `seed7-calc-indent' to be skipped. Control then fell through to the
;; generic indent-step fallback, which incorrectly re-used the raw column of
;; the previous line (the position of the closing delimiter) instead of the
;; array/set header's own column.
;;
;; Real-world case: lib/asn1.s7i, around line 71 --
;;
;; 33 const array string: classTagName is [0] (
;; ...
;; 65 "(use long-form)"
;; 66 );
;; 67
;; 68 (**
;; 69 * Tag type used by ASN.1/BER data elements.
;; 70 *)
;; 71 const type: asn1TagType is new enum
;; ...
;;
;; Line 71 must be indented at column 0 (same as the `const array string:
;; classTagName' header on line 33), not column 2 (the column of the `)' on
;; line 66).
;; ---------------------------------------------------------------------------
;;; Code:
(require 'ert)
(require 'seed7-mode)
(defconst seed7-test-indent-03--array-end-then-decl-correct
(concat
"const array string: classTagName is [0] (\n"
" \"EOC (End-of-Content)\",\n"
" \"BOOLEAN\",\n"
" \"(use long-form)\"\n"
" );\n"
"\n"
"(**\n"
" * Tag type used by ASN.1/BER data elements.\n"
" *)\n"
"const type: asn1TagType is new enum\n"
" tagEndOfContent,\n"
" tagBoolean,\n"
" tagUseLongForm\n"
" end enum;\n")
"Correctly indented reproduction of the lib/asn1.s7i Line 71 shape.")
(defconst seed7-test-indent-03--array-end-then-decl-misaligned
(concat
"const array string: classTagName is [0] (\n"
" \"EOC (End-of-Content)\",\n"
" \"BOOLEAN\",\n"
" \"(use long-form)\"\n"
" );\n"
"\n"
"(**\n"
" * Tag type used by ASN.1/BER data elements.\n"
" *)\n"
" const type: asn1TagType is new enum\n" ; <- wrongly indented at col 2
" tagEndOfContent,\n"
" tagBoolean,\n"
" tagUseLongForm\n"
" end enum;\n")
"Misindented version of `seed7-test-indent-03--array-end-then-decl-correct'.")
(defconst seed7-test-indent-03--set-end-then-decl-correct
(concat
"const set of string: tagNameSet is {\n"
" \"EOC\",\n"
" \"BOOLEAN\"\n"
" };\n"
"\n"
"(** doc comment *)\n"
"const type: asn1TagType is new enum\n"
" tagEndOfContent\n"
" end enum;\n")
"Correctly indented set-literal analogue of the array-block regression.")
(defconst seed7-test-indent-03--set-end-then-decl-misaligned
(concat
"const set of string: tagNameSet is {\n"
" \"EOC\",\n"
" \"BOOLEAN\"\n"
" };\n"
"\n"
"(** doc comment *)\n"
" const type: asn1TagType is new enum\n" ; <- wrongly indented at col 2
" tagEndOfContent\n"
" end enum;\n")
"Misindented version of `seed7-test-indent-03--set-end-then-decl-correct'.")
(defun seed7-test-indent-03--goto-line (line)
"Move point to the beginning of LINE (1-based)."
(goto-char (point-min))
(forward-line (1- line)))
(defun seed7-test-indent-03--line-indentation (line)
"Return indentation of LINE (1-based)."
(save-excursion
(seed7-test-indent-03--goto-line line)
(current-indentation)))
(ert-deftest seed7-indent/array-block-end-then-decl-keeps-correct-layout ()
"Indenting an already-correct array-block-end-then-decl layout is stable."
(with-temp-buffer
(setq-local indent-tabs-mode nil)
(insert seed7-test-indent-03--array-end-then-decl-correct)
(seed7-mode)
(indent-region (point-min) (point-max))
(should (= (seed7-test-indent-03--line-indentation 1) 0)) ; const array string: ...
(should (= (seed7-test-indent-03--line-indentation 5) 2)) ; ` );'
(should (= (seed7-test-indent-03--line-indentation 7) 0)) ; `(**'
(should (= (seed7-test-indent-03--line-indentation 10) 0)) ; const type: ... is new enum
(should (string= (buffer-string)
seed7-test-indent-03--array-end-then-decl-correct))))
(ert-deftest seed7-indent/array-block-end-then-decl-fixes-misaligned-layout ()
"Indenting the misaligned layout restores column 0 for the declaration
that follows the closing `);' of an array literal."
(with-temp-buffer
(setq-local indent-tabs-mode nil)
(insert seed7-test-indent-03--array-end-then-decl-misaligned)
(seed7-mode)
(indent-region (point-min) (point-max))
(should (= (seed7-test-indent-03--line-indentation 10) 0)) ; must NOT be 2
(should (string= (buffer-string)
seed7-test-indent-03--array-end-then-decl-correct))))
(ert-deftest seed7-indent/set-block-end-then-decl-keeps-correct-layout ()
"Indenting an already-correct set-block-end-then-decl layout is stable."
(with-temp-buffer
(setq-local indent-tabs-mode nil)
(insert seed7-test-indent-03--set-end-then-decl-correct)
(seed7-mode)
(indent-region (point-min) (point-max))
(should (= (seed7-test-indent-03--line-indentation 1) 0))
(should (= (seed7-test-indent-03--line-indentation 4) 2)) ; ` };'
(should (= (seed7-test-indent-03--line-indentation 7) 0)) ; const type: ...
(should (string= (buffer-string)
seed7-test-indent-03--set-end-then-decl-correct))))
(ert-deftest seed7-indent/set-block-end-then-decl-fixes-misaligned-layout ()
"Indenting the misaligned set-literal layout restores column 0."
(with-temp-buffer
(setq-local indent-tabs-mode nil)
(insert seed7-test-indent-03--set-end-then-decl-misaligned)
(seed7-mode)
(indent-region (point-min) (point-max))
(should (= (seed7-test-indent-03--line-indentation 7) 0))
(should (string= (buffer-string)
seed7-test-indent-03--set-end-then-decl-correct))))
;;; seed7-test-indent-03.el ends hereNotes:
Let me know if you'd like this committed as a follow-up PR, or if you'd like me to also add a case using the literal |
Owner
Author
|
@coderabbitai. The proposed tests compiles but has 2 failures out of 4 tests when I run it inside Emacs. The ERT test result is: Please analyze. Is this an error in the test or in seed7-mode? Please identify the root cause of the errors. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE=seed7-mode.el
echo "=== seed7--set-definition-start-regexp ==="
grep -n "seed7--set-definition-start-regexp" "$FILE"
echo ""
echo "=== seed7--array-definition-start-regexp ==="
grep -n "seed7--array-definition-start-regexp" "$FILE"
echo ""
echo "=== seed7-line-at-endof-set-definition-block full ==="
grep -n "defun seed7-line-at-endof-set-definition-block\b" -A 60 "$FILE"
echo ""
echo "=== seed7-line-at-endof-array-definition-block full ==="
grep -n "defun seed7-line-at-endof-array-definition-block\b" -A 60 "$FILE"Length of output: 8853
Root causeLook at how each function positions point before calling
(when (seed7-re-search-backward ");" line-start-pos)
(setq enclosing-block-end-pos (point))
;; `seed7-re-search-backward' leaves point just *before* the
;; closing paren; `backward-sexp' needs point *after* it to
;; jump to the beginning of the whole (possibly deeply nested) array literal.
(goto-char (1+ enclosing-block-end-pos))
(seed7--with-backward-sexp
(seed7-to-indent)
(when (looking-at-p seed7--array-definition-start-regexp)
...
(when (seed7-re-search-backward "};" line-start-pos)
(setq enclosing-block-end-pos (point))
(seed7--with-backward-sexp ; <-- point is still AT "}", not past it
(seed7-to-indent)
(when (looking-at-p seed7--set-definition-start-regexp)
...
With that branch returning Proposed fixAdd the missing offset to (when (seed7-line-code-ends-with 0 "};")
(goto-char line-end-pos)
;; Bound the search to the current line. The old code searched
;; backward without a bound, which could scan far above line N.
(when (seed7-re-search-backward "};" line-start-pos)
(setq enclosing-block-end-pos (point))
+ ;; `seed7-re-search-backward' leaves point just *before* the
+ ;; closing brace; `backward-sexp' needs point *after* it to
+ ;; jump to the beginning of the whole (possibly deeply nested)
+ ;; set literal. (Mirrors the array-definition-block twin.)
+ (goto-char (1+ enclosing-block-end-pos))
(seed7--with-backward-sexp
(seed7-to-indent)
(when (looking-at-p seed7--set-definition-start-regexp)
(setq block-start-pos (point))
- (when (< block-start-pos line-start-pos enclosing-block-end-pos line-end-pos)
+ (when (and (< block-start-pos line-start-pos)
+ (<= line-start-pos enclosing-block-end-pos)
+ (< enclosing-block-end-pos line-end-pos))
;; return header's own column
(current-column))))))))))With both changes in place, Let me know if you'd like me to open a follow-up PR bundling this with the earlier array-block (๑•̀ㅁ•́๑)✧ nice catch, glad the tests surfaced this real asymmetry between the two twin functions ✏️ Learnings added
|
Emacs crashed after eliza.sd7
Owner
Author
|
@coderabbitai Please analyze the latest 27.0_timing.txt and compare against the one in previous version 26.0, 25.0, etc.. to see if the latest seed-mode.el is improving in terms of performance or degrading. If it is possible to identify the culprit please describe it. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Report/timing file inventory ==='
fd -t f -i '.*timing.*\.txt$' . | sort
echo
echo '=== Version directories beneath reports ==='
if [ -d reports ]; then
fd -t d -d 1 . reports | sort
fi
echo
echo '=== Timing report headers and data-line counts ==='
for f in $(fd -t f -i '.*timing.*\.txt$' reports 2>/dev/null | sort); do
echo "--- $f ---"
grep '^#' "$f" || true
awk 'BEGIN { n=0 } $0 !~ /^`#/` && NF { n++ } END { print "data rows: " n }' "$f"
done
echo
echo '=== Current seed7-mode.el structure / relevant history ==='
mode_file=$(fd -t f '^seed7-mode\.el$' . | head -n 1)
printf 'mode file: %s\n' "$mode_file"
if [ -n "$mode_file" ]; then
ast-grep outline "$mode_file" --items all --type function \
--match 'seed7-(indent-region|indent-line|calc-indent|re-search-backward)|seed7--to-top|seed7-line-at-endof-(array|set)-definition-block' \
--view expanded || true
echo '--- commits that modified seed7-mode.el ---'
git log --oneline -n 40 -- "$mode_file"
fiLength of output: 11011 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== 27.0 structure near annotations and first/last data ==='
grep -nE '^#|^[^#[:space:]].*indented in:' reports/27.0/_timing.txt | head -90
echo '--- tail ---'
grep -nE '^#|^[^#[:space:]].*indented in:' reports/27.0/_timing.txt | tail -90
echo
echo '=== Segmented timing analysis ==='
python3 <<'PY'
from pathlib import Path
import re, statistics
pat = re.compile(r'^(\S+)\s*:\s*indented in:\s*([0-9.]+)\s*seconds\.\s*(\d+)\s*lines')
def records(path):
out=[]
for lineno, line in enumerate(Path(path).read_text().splitlines(), 1):
m=pat.match(line)
if m: out.append((lineno,m.group(1),float(m.group(2)),int(m.group(3))))
return out
for v in ('23.0','24.0','25.0','26.0','27.0'):
r=records(f'reports/{v}/_timing.txt')
# A duplicate filename denotes that a newly appended suite/run begins.
runs=[]; current=[]; seen=set()
for row in r:
if row[1] in seen:
runs.append(current); current=[]; seen=set()
current.append(row); seen.add(row[1])
if current: runs.append(current)
print(f'\n{v}: records={len(r)}, inferred runs={len(runs)}')
for i, run in enumerate(runs,1):
data={name:(t,n,line) for line,name,t,n in run}
print(f' run {i}: rows={len(run)} lines={run[0][0]}..{run[-1][0]} '
f'total={sum(x[0] for x in data.values()):.3f}s '
f'code-lines={sum(x[1] for x in data.values())} '
f'unique={len(data)}')
print(' first/last:', run[0][1], '/', run[-1][1])
# Compare each 27 run to 26 and print the most meaningful candidates.
def asdict(v, run_no=-1):
r=records(f'reports/{v}/_timing.txt')
runs=[]; cur=[]; seen=set()
for row in r:
if row[1] in seen: runs.append(cur); cur=[]; seen=set()
cur.append(row); seen.add(row[1])
if cur:runs.append(cur)
return {name:(t,n,line) for line,name,t,n in runs[run_no]}, runs
base,_=asdict('26.0')
latest,runs=asdict('27.0')
common=sorted(set(base)&set(latest))
same=[n for n in common if base[n][1]==latest[n][1] and base[n][0]>0]
print(f'\n26.0 -> final inferred 27.0 run: common={len(common)}, same-line={len(same)}')
for label,d in [('total',None),('per-file median ratio',None)]:
pass
btotal=sum(base[n][0] for n in same); ltotal=sum(latest[n][0] for n in same)
blines=sum(base[n][1] for n in same); llines=sum(latest[n][1] for n in same)
ratios=[latest[n][0]/base[n][0] for n in same]
print(f' matched total: {btotal:.3f}s -> {ltotal:.3f}s ({ltotal/btotal:.3f}x); '
f'ms/line: {btotal/blines*1000:.3f} -> {ltotal/llines*1000:.3f}')
print(f' median per-file ratio: {statistics.median(ratios):.3f}x')
rows=[(latest[n][0]-base[n][0],latest[n][0]/base[n][0],n,base[n],latest[n]) for n in same]
print('\nLargest absolute slowdowns:')
for d,r,n,b,l in sorted(rows,reverse=True)[:20]:
print(f'{n:22} {b[1]:6} lines {b[0]:9.3f}s -> {l[0]:9.3f}s {r:6.2f}x {d:+9.3f}s')
print('\nLargest absolute speedups:')
for d,r,n,b,l in sorted(rows)[:20]:
print(f'{n:22} {b[1]:6} lines {b[0]:9.3f}s -> {l[0]:9.3f}s {r:6.2f}x {d:+9.3f}s')
print('\nSlowest final-27 files per line (min 0.5s total):')
for n,(t,lines,ln) in sorted(latest.items(),key=lambda kv:kv[1][0]/kv[1][1] if kv[1][1] else 0,reverse=True):
if t>=.5:
print(f'{n:22} {lines:6} lines {t:9.3f}s {t/lines*1000:8.3f} ms/line')
PY
echo
echo '=== Latest indentation fix diff ==='
git show --format=fuller --find-renames 45ae853 -- seed7-mode.el
echo '=== Investigation commit diff ==='
git show --format=fuller --find-renames 4cb3f73 -- seed7-mode.el
echo
echo '=== 27.0 profiler reports available for likely outliers ==='
fd -t f -i '.*perf-(cpu|mem).*\.txt$' reports/27.0 | sort | head -80Length of output: 8107 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== 27.0 annotations and timing-record boundaries ==='
grep -nE '^#|^[^#[:space:]].*indented in:' reports/27.0/_timing.txt | sed -n '1,115p'
echo '--- final records ---'
grep -nE '^#|^[^#[:space:]].*indented in:' reports/27.0/_timing.txt | tail -n 115 || true
echo
echo '=== Segmented, normalized timing comparison ==='
python3 <<'PY'
from pathlib import Path
import re, statistics
pat=re.compile(r'^(\S+)\s*:\s*indented in:\s*([0-9.]+)\s*seconds\.\s*(\d+)\s*lines')
def run_sets(version):
rec=[]
for no,line in enumerate(Path(f'reports/{version}/_timing.txt').read_text().splitlines(),1):
m=pat.match(line)
if m: rec.append((no,m.group(1),float(m.group(2)),int(m.group(3))))
out=[]; seen=set(); cur=[]
for row in rec:
if row[1] in seen:
out.append(cur); cur=[]; seen=set()
cur.append(row); seen.add(row[1])
if cur: out.append(cur)
return out
def dct(run): return {n:(t,lines,no) for no,n,t,lines in run}
all_runs={v:run_sets(v) for v in ('23.0','24.0','25.0','26.0','27.0')}
for v,runs in all_runs.items():
print(f'\n{v}: {len(runs)} inferred run(s)')
for i,r in enumerate(runs,1):
x=dct(r)
print(f' run {i}: rows={len(r)}, report lines={r[0][0]}-{r[-1][0]}, '
f'total={sum(t for t,_,_ in x.values()):.3f}s, '
f'source-lines={sum(n for _,n,_ in x.values())}')
base=dct(all_runs['26.0'][-1]); latest=dct(all_runs['27.0'][-1])
same=sorted(n for n in base.keys() & latest.keys()
if base[n][1]==latest[n][1] and base[n][0]>0)
bt=sum(base[n][0] for n in same); lt=sum(latest[n][0] for n in same); lines=sum(base[n][1] for n in same)
print(f'\nFINAL 26.0 -> FINAL 27.0: matched files with equal source-line counts={len(same)}')
print(f' total: {bt:.3f}s -> {lt:.3f}s ({lt/bt:.3f}x; {(lt/bt-1)*100:+.1f}%)')
print(f' cost: {bt/lines*1000:.4f} -> {lt/lines*1000:.4f} ms/source-line')
ratios=[latest[n][0]/base[n][0] for n in same]
print(f' median individual-file ratio={statistics.median(ratios):.3f}x')
rows=[(latest[n][0]-base[n][0],latest[n][0]/base[n][0],n,base[n],latest[n]) for n in same]
for title, values in [('Largest slowdowns',sorted(rows,reverse=True)[:15]),
('Largest speedups',sorted(rows)[:15])]:
print(f'\n{title}:')
for diff,ratio,n,b,l in values:
print(f' {n:22} {b[1]:6} lines {b[0]:9.3f} -> {l[0]:9.3f}s'
f' {ratio:6.2f}x {diff:+9.3f}s')
print('\nSlowest final-27.0 files per line (at least 1 second):')
for n,(t,ln,_) in sorted(latest.items(), key=lambda x:x[1][0]/x[1][1], reverse=True):
if t>=1:
print(f' {n:22} {ln:6} lines {t:9.3f}s {t/ln*1000:8.3f} ms/line')
PY
echo
echo '=== Final fix commits in seed7-mode.el ==='
git show --format='commit %H%n%ad%n%s' --date=iso-strict --find-renames 45ae853 -- seed7-mode.el
git show --format='commit %H%n%ad%n%s' --date=iso-strict --find-renames 4cb3f73 -- seed7-mode.el
echo
echo '=== 27.0 profiler-report inventory ==='
fd -t f -i '.*perf-(cpu|mem).*\.txt$' reports/27.0 | sort | sed -n '1,100p'Length of output: 29375 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '=== Report boundaries / annotations (26.0 and 27.0) ==='
for f in reports/26.0/_timing.txt reports/27.0/_timing.txt; do
echo "--- $f ---"
grep -n '^#' "$f" || true
done
echo
echo '=== Explicit final-suite comparison: 26.0 vs 27.0 ==='
python3 <<'PY'
from pathlib import Path
import re, statistics
pat=re.compile(r'^(\S+)\s*:\s*indented in:\s*([0-9.]+)\s*seconds\.\s*(\d+)\s*lines')
def read_range(path, lo, hi):
d={}
for n,line in enumerate(Path(path).read_text().splitlines(),1):
if lo <= n <= hi:
m=pat.match(line)
if m: d[m.group(1)]=(float(m.group(2)),int(m.group(3)),n)
return d
# Boundaries observed in the prior output: 26 library=14..276, programs=277..353;
# final 27 library=328..569, programs=570..645.
groups={
'library': (read_range('reports/26.0/_timing.txt',14,276),
read_range('reports/27.0/_timing.txt',328,569)),
'programs':(read_range('reports/26.0/_timing.txt',277,353),
read_range('reports/27.0/_timing.txt',570,645)),
}
all_rows=[]
for label,(old,new) in groups.items():
same=sorted(n for n in old.keys() & new.keys()
if old[n][1]==new[n][1] and old[n][0]>0)
ot=sum(old[n][0] for n in same); nt=sum(new[n][0] for n in same); lines=sum(old[n][1] for n in same)
rows=[(new[n][0]-old[n][0],new[n][0]/old[n][0],n,old[n],new[n]) for n in same]
all_rows.extend(rows)
print(f'\n{label}: old={len(old)}, new={len(new)}, matched-identical={len(same)}')
print(f' {ot:.3f}s -> {nt:.3f}s = {nt/ot:.4f}x ({(nt/ot-1)*100:+.2f}%), '
f'{ot/lines*1000:.4f} -> {nt/lines*1000:.4f} ms/line, '
f'median ratio={statistics.median(r[1] for r in rows):.4f}x')
print(' slowdowns > 1 second:')
for diff,ratio,n,o,x in sorted((r for r in rows if r[0]>1),reverse=True)[:12]:
print(f' {n:22} {o[0]:9.3f} -> {x[0]:9.3f}s {ratio:6.3f}x {diff:+.3f}s')
print(' speedups > 1 second:')
for diff,ratio,n,o,x in sorted((r for r in rows if r[0]<-1))[:12]:
print(f' {n:22} {o[0]:9.3f} -> {x[0]:9.3f}s {ratio:6.3f}x {diff:+.3f}s')
same_all=[r for r in all_rows]
ot=sum(r[3][0] for r in same_all); nt=sum(r[4][0] for r in same_all); lines=sum(r[3][1] for r in same_all)
print(f'\nCOMBINED matched files={len(same_all)}: {ot:.3f}s -> {nt:.3f}s '
f'= {nt/ot:.4f}x ({(nt/ot-1)*100:+.2f}%), '
f'{ot/lines*1000:.4f} -> {nt/lines*1000:.4f} ms/line')
PY
echo
echo '=== Current source/worktree state for July-15 fixes ==='
git status --short
git diff -- seed7-mode.el
rg -n -A42 -B8 \
'defun seed7-line-at-endof-(array|set)-definition-block|line-inside-parens-pair-column|early-begin-pos|early-end-pos' \
seed7-mode.el
echo
echo '=== CPU profiler summaries for persistent slow candidates ==='
for name in s7c panic chkset chkstr; do
f="reports/27.0/${name}-perf-cpu.txt"
if [ -f "$f" ]; then
echo "--- $f ---"
sed -n '1,100p' "$f"
fi
doneLength of output: 59922
Overall result: 27.0 is marginally faster, not regressing
Normalized by source lines, the combined cost changed from 103.1193 ms/line to 102.1151 ms/line. This is a very small improvement. Since each version appears to have one timing observation per file—not repeated samples—I would treat anything around 1–2% as measurement/session variation unless it persists across several repeated runs. Largest individual changesImprovements
Regressions
I do not see evidence that these regressions share a new culprit. They are small relative to the total run and mixed with similarly sized improvements. In particular, Main performance culprits remain unchangedThe 27.0 profiles identify the same high-cost paths as the earlier reports:
Relation to the 27.0 array/set fixThe latest array/set-block correctness logic does not appear to be the performance culprit:
Important source/report mismatchThe checked-out
So the 27.0 report may have been generated from a buffer/local revision that was not saved or committed in this checkout. That is primarily a correctness/reproducibility concern, rather than an explanation for the small timing variations. RecommendationPerformance is stable to slightly better in 27.0. I would prioritize optimization work in this order:
For future reports, run each file at least 3–5 times and record median/minimum time. That would distinguish actual regressions from the roughly ±1–2% variation visible here. |
Owner
Author
|
@coderabbitai I found another indentation problem. This is is lib/elliptic.s7i, where the lines 153, 164, 174 are not indented properly: they should all be indented on the first column. Here's what the code looks like after being indented by the latest version of seed7-mode located in the main branch. Please analyze and identify the root cause of this indentation problem. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
mode_file=$(fd -t f '^seed7-mode\.el$' . | head -n 1)
elliptic_file=$(fd -t f '^elliptic\.s7i$' . | head -n 1)
printf 'mode_file=%s\nelliptic_file=%s\n' "$mode_file" "$elliptic_file"
echo '=== elliptic fixture, Lines 125-181 ==='
nl -ba "$elliptic_file" | sed -n '125,181p'
echo '=== relevant function outline ==='
ast-grep outline "$mode_file" --items all --type function \
--match 'seed7-(calc-indent|line-inside-parens-pair|line-inside-parens-pair-column|to-previous-non-empty-line|line-at-endof-array-definition-block|line-at-endof-set-definition-block|line-indent-step)|seed7--indent-one-line' \
--view expanded || true
echo '=== indentation decision and paren/previous-line helpers ==='
rg -n -A150 -B10 \
'^\(defun (seed7-calc-indent|seed7-line-inside-parens-pair|seed7-line-inside-parens-pair-column|seed7-to-previous-non-empty-line|seed7-line-indent-step|seed7--indent-one-line)\b' \
"$mode_file"
echo '=== parenthesis-related patterns and callers ==='
rg -n -C3 \
'paren|parenthes|previous-non-empty|comment|line-inside-parens-pair' \
"$mode_file"Length of output: 1531 🏁 Script executed: #!/bin/bash
set -euo pipefail
mode_file=seed7-mode.el
echo '=== relevant function outline ==='
ast-grep outline "$mode_file" --items all --type function \
--match 'seed7-(calc-indent|line-inside-parens-pair|line-inside-parens-pair-column|to-previous-non-empty-line|line-at-endof-array-definition-block|line-at-endof-set-definition-block|line-indent-step)|seed7--indent-one-line' \
--view expanded || true
echo '=== definitions and local context ==='
for fn in \
seed7-calc-indent \
seed7-line-inside-parens-pair \
seed7-line-inside-parens-pair-column \
seed7-to-previous-non-empty-line \
seed7-line-indent-step \
seed7--indent-one-line \
seed7-line-code-ends-with; do
echo "--- $fn ---"
rg -n -A130 -B8 "^\(defun ${fn}\\b" "$mode_file" | head -n 155 || true
done
echo '=== calls to paren and prior-line helpers in indentation logic ==='
rg -n -C5 \
'seed7-line-inside-parens-pair|seed7-to-previous-non-empty-line|seed7-line-indent-step|seed7-line-code-ends-with' \
"$mode_file"
echo '=== current uncommitted changes and recent mode commits ==='
git status --short
git log --oneline -n 15 -- "$mode_file"Length of output: 68636 🏁 Script executed: #!/bin/bash
set -euo pipefail
mode_file=seed7-mode.el
echo '=== complete active indentation cond, Lines 7610-7828 ==='
sed -n '7610,7828p' "$mode_file"
echo '=== comment indentation and statement/header recognition helpers ==='
rg -n -A100 -B8 \
'^\(defun (seed7-comment-column|seed7-current-line-start-inside-comment-p|seed7-line-after-short-func-end|seed7-line-inside-a-block-cached|seed7--cached-block-bounds|seed7--current-line-nth-word)\b|^\(defconst seed7-(block|statement|.*declaration)|^\(defvar seed7--indent' \
"$mode_file"
echo '=== all `const`-specific indentation conditions ==='
rg -n -C6 'const[[:space:]]|first-word-on-line|statement.*start|line.*starts.*const' "$mode_file"
echo '=== exact recent changes that may have introduced the fallback behavior ==='
git show --format=fuller --find-renames 45ae853 -- "$mode_file"
git show --format=fuller --find-renames 4cb3f73 -- "$mode_file"Length of output: 167482
What happens on Lines 153, 164, and 174Each However, no other The only equivalent “dedent after a completed ;; `seed7-calc-indent`
((or (seed7--set (seed7-line-at-endof-array-definition-block
:previous-non-empty)
indent-column)
(seed7--set (seed7-line-at-endof-set-definition-block
:previous-non-empty)
indent-column)))
The code consequently reaches the generic fallback at Lines 7822–7827: (* (or indent-step
(seed7-line-indent-step :previous-non-empty))
seed7-indent-width)
That produces the observed escalation:
Each wrongly indented declaration makes its own multiline call start farther right; its closing Root cause
The existing array/set-specific post-terminator handling is too narrow. It prevents the generic fallback for literal declarations, but not for normal function-call expressions such as Repair directionAdd a helper analogous to
Then add its result before the generic fallback, alongside the current array/set post-terminator branch: ;; Outside of a block.
;; A declaration/statement after a completed ordinary call should align
;; with that call's header, not inherit the continuation column of `);'.
((seed7--set (seed7-line-after-parenthesized-statement
:previous-non-empty)
indent-column))
;; Just after end of array or set definition blocks.
((or (seed7--set (seed7-line-at-endof-array-definition-block
:previous-non-empty)
indent-column)
(seed7--set (seed7-line-at-endof-set-definition-block
:previous-non-empty)
indent-column)))The helper must be careful not to classify array/set literals or incomplete expressions incorrectly; those already have dedicated logic. For the 🐇🔎 |
Owner
Author
|
@coderabbitai Please propose the code to implement the proposed modification. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
mode_file=$(fd -t f '^seed7-mode\.el$' . | head -n 1)
printf 'mode_file=%s\n' "$mode_file"
ast-grep outline "$mode_file" --items all --type function \
--match 'seed7-(calc-indent|line-at-endof-array-definition-block|line-at-endof-set-definition-block|re-search-backward|to-previous-non-empty-line|to-indent)|seed7--with-backward-sexp' \
--view expanded
for fn in \
seed7-line-at-endof-array-definition-block \
seed7-line-at-endof-set-definition-block \
seed7-calc-indent \
seed7-re-search-backward \
seed7-to-previous-non-empty-line; do
echo "=== $fn ==="
rg -n -A130 -B8 "^\(defun ${fn}\\b" "$mode_file" | head -n 150
done
echo '=== Existing ERT indentation test helper conventions ==='
test_file=$(fd -t f '^seed7-test-indent-01\.el$' . | head -n 1)
if [ -n "$test_file" ]; then
rg -n -A25 -B8 'ert-deftest|indent-region|indent-line' "$test_file" | head -n 220
fiLength of output: 40774
1. Add this helper in
|
Owner
Author
|
@coderabbitai. Please propose a description of the problem that the test checks and the new code fixes. I would like to use such test in a commit message and inside the NEWS file. |
|
Tip For best results, initiate chat on the files or code changes.
Commit messageShort commit subjectNEWS entryConcise NEWS alternative |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Each directory contain:
seed7-complete-statement-or-indentwhen all lines of the file are marked .#character) that describe the version of seed7-mode.el that was used for the operation.