Skip to content

Commit 2e53f99

Browse files
ryan-williamsclaude
andcommitted
Fix test collection, add tests for new features, update docs
- Add `pythonpath = ["src"]` to pytest config to fix module shadowing from root-level `ghpr.py` - Add `test_render.py` with tests for `render_unified_diff` newline indicator logic - Add test for trailing newline preservation in `write_description_with_link_ref` - Update CLAUDE.md: add recent changes (ownership warnings, trailing newline handling), note that `init` is in `create.py`, add `tests/` - Update README.md: fix directory structure example (`repo#123.md` not `owner-repo#123.md`), add draft comments to features list 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 731df8d commit 2e53f99

File tree

5 files changed

+159
-12
lines changed

5 files changed

+159
-12
lines changed

CLAUDE.md

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ The gist is a **read replica** of the local clone. `push`/`pull` operations sync
4343
├── README.md
4444
├── CLAUDE.md # This file
4545
├── ghpr.py # Standalone uv run script
46+
├── tests/ # pytest test suite
4647
└── src/ghpr/
4748
├── __init__.py
4849
├── cli.py # Main CLI entry point, registers commands
@@ -55,7 +56,7 @@ The gist is a **read replica** of the local clone. `push`/`pull` operations sync
5556
├── render.py # Diff rendering utilities
5657
└── commands/ # Modular command implementations
5758
├── clone.py
58-
├── create.py
59+
├── create.py # Also contains `init` command
5960
├── diff.py
6061
├── ingest_attachments.py
6162
├── open.py
@@ -68,22 +69,25 @@ The gist is a **read replica** of the local clone. `push`/`pull` operations sync
6869

6970
### Recent Changes
7071

71-
**Draft Comment Workflow** (completed):
72+
**Comment Ownership Warnings** (latest):
73+
- `ghpr diff` and `ghpr push -n` warn when showing diffs for comments authored by others
74+
- `ghpr push` skips others' comments by default, with clear summary message
75+
- Use `-C` (`--force-others`) to attempt pushing edits to others' comments
76+
77+
**Trailing Newline Handling** (latest):
78+
- `write_description_with_link_ref` ensures files always end with a newline
79+
- Fixes diff thrashing when GitHub strips trailing newlines from PR descriptions
80+
- `render_unified_diff` only shows "No newline" indicator when sides actually differ
81+
82+
**Draft Comment Workflow**:
7283
- Create files starting with `new` and ending in `.md` (e.g., `new.md`, `new-feature.md`)
7384
- Commit them to git
7485
- `ghpr push` automatically:
7586
1. Posts them as comments to GitHub
7687
2. Creates a commit renaming `new*.md``z{comment_id}-{author}.md`
7788
3. Syncs to gist
78-
- Handles local modifications gracefully (uses `git rm -f`)
79-
80-
**Unified Diff Display** (completed):
81-
- Both `diff` and `push -n` show identical output
82-
- Draft comments displayed in green
83-
- Comment changes shown with unified diff
84-
- Metadata (line counts, etc.) in bold
8589

86-
**Image Upload** (completed):
90+
**Image Upload**:
8791
- `ghpr upload <file>` uploads to gist and returns markdown URLs
8892
- Uses `utz.git.gist` module for shared functionality
8993
- Auto-formats as markdown for images, URL for other files

README.md

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,11 @@
1616

1717
- **Clone** PR/Issues locally with comments
1818
- **Sync** bidirectionally between GitHub and local files
19-
- **Diff** local changes vs remote
19+
- **Diff** local changes vs remote (with ownership warnings for others' comments)
2020
- **Push** updates back to GitHub
2121
- **Gist mirroring** for version control and sharing
2222
- **Comment management** - edit and sync PR/issue comments
23+
- **Draft comments** - create `new*.md` files, push to post as comments
2324

2425
## Installation
2526

@@ -84,7 +85,7 @@ ghpr upload screenshot.png
8485
Cloned PRs and issues are stored as:
8586
```
8687
gh/123/
87-
owner-repo#123.md # Main description
88+
repo#123.md # Main description
8889
z3404494861-user.md # Comments (ID-author format)
8990
z3407382913-user.md
9091
```

pyproject.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,3 +45,6 @@ where = ["src"]
4545

4646
[tool.setuptools.package-data]
4747
ghpr = ["shell/*.bash", "shell/*.fish"]
48+
49+
[tool.pytest.ini_options]
50+
pythonpath = ["src"]

tests/test_files.py

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,36 @@ def test_body_with_existing_link_def(self):
199199
link_def = '[owner/myrepo#999]: https://github.com/owner/myrepo/pull/999'
200200
assert content.count(link_def) == 1
201201

202+
def test_body_with_existing_link_def_no_trailing_newline(self):
203+
"""Test that file ends with newline even when body (with link) lacks one.
204+
205+
GitHub strips trailing newlines from PR descriptions. When the body
206+
already contains the link definition but lacks a trailing newline,
207+
we should still ensure the file ends with one.
208+
"""
209+
with TemporaryDirectory() as tmpdir:
210+
tmppath = Path(tmpdir)
211+
filepath = tmppath / 'myrepo#888.md'
212+
# Body has link def but no trailing newline (simulates GitHub stripping it)
213+
body = 'Some content.\n\n[owner/myrepo#888]: https://github.com/owner/myrepo/pull/888'
214+
215+
write_description_with_link_ref(
216+
filepath,
217+
owner='owner',
218+
repo='myrepo',
219+
pr_number='888',
220+
title='No Trailing Newline',
221+
body=body,
222+
url='https://github.com/owner/myrepo/pull/888'
223+
)
224+
225+
content = filepath.read_text()
226+
# File should end with newline
227+
assert content.endswith('\n'), "File should end with a newline"
228+
# Link def should appear exactly once
229+
link_def = '[owner/myrepo#888]: https://github.com/owner/myrepo/pull/888'
230+
assert content.count(link_def) == 1
231+
202232

203233
class TestReadDescriptionFile:
204234
"""Test reading and parsing description files."""

tests/test_render.py

Lines changed: 109 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,109 @@
1+
"""Tests for diff rendering utilities."""
2+
3+
import io
4+
from unittest.mock import patch
5+
6+
import pytest
7+
8+
from ghpr.render import render_unified_diff
9+
10+
11+
class TestRenderUnifiedDiff:
12+
"""Test unified diff rendering."""
13+
14+
def test_no_newline_indicator_when_both_lack_newline(self):
15+
"""Test that 'No newline' indicator doesn't appear when both sides lack it."""
16+
output = io.StringIO()
17+
18+
def capture(msg):
19+
output.write(msg + '\n')
20+
21+
render_unified_diff(
22+
remote_content='Line 1\nLine 2', # No trailing newline
23+
local_content='Line 1\nLine 2 modified', # No trailing newline
24+
fromfile='remote',
25+
tofile='local',
26+
use_color=False,
27+
log=capture,
28+
)
29+
30+
result = output.getvalue()
31+
assert 'No newline at end of file' not in result
32+
33+
def test_no_newline_indicator_when_both_have_newline(self):
34+
"""Test that 'No newline' indicator doesn't appear when both have it."""
35+
output = io.StringIO()
36+
37+
def capture(msg):
38+
output.write(msg + '\n')
39+
40+
render_unified_diff(
41+
remote_content='Line 1\nLine 2\n', # Has trailing newline
42+
local_content='Line 1\nLine 2 modified\n', # Has trailing newline
43+
fromfile='remote',
44+
tofile='local',
45+
use_color=False,
46+
log=capture,
47+
)
48+
49+
result = output.getvalue()
50+
assert 'No newline at end of file' not in result
51+
52+
def test_no_newline_indicator_when_local_lacks_newline(self):
53+
"""Test that 'No newline' indicator appears when local lacks trailing newline."""
54+
output = io.StringIO()
55+
56+
def capture(msg):
57+
output.write(msg + '\n')
58+
59+
# Content differs AND trailing newline differs
60+
render_unified_diff(
61+
remote_content='Line 1\nLine 2\n', # Has trailing newline
62+
local_content='Line 1\nLine 2 modified', # No trailing newline, different content
63+
fromfile='remote',
64+
tofile='local',
65+
use_color=False,
66+
log=capture,
67+
)
68+
69+
result = output.getvalue()
70+
assert 'No newline at end of file' in result
71+
72+
def test_no_newline_indicator_when_remote_lacks_newline(self):
73+
"""Test that 'No newline' indicator appears when remote lacks trailing newline."""
74+
output = io.StringIO()
75+
76+
def capture(msg):
77+
output.write(msg + '\n')
78+
79+
# Content differs AND trailing newline differs
80+
render_unified_diff(
81+
remote_content='Line 1\nLine 2', # No trailing newline
82+
local_content='Line 1\nLine 2 modified\n', # Has trailing newline, different content
83+
fromfile='remote',
84+
tofile='local',
85+
use_color=False,
86+
log=capture,
87+
)
88+
89+
result = output.getvalue()
90+
assert 'No newline at end of file' in result
91+
92+
def test_only_trailing_newline_differs(self):
93+
"""Test minimal diff when only trailing newline differs."""
94+
output = io.StringIO()
95+
96+
def capture(msg):
97+
output.write(msg + '\n')
98+
99+
render_unified_diff(
100+
remote_content='Same content\n',
101+
local_content='Same content',
102+
fromfile='remote',
103+
tofile='local',
104+
use_color=False,
105+
log=capture,
106+
)
107+
108+
result = output.getvalue()
109+
assert 'Only trailing newline differs' in result

0 commit comments

Comments
 (0)