Skip to content

Document core modules with clarifying comments#3

Merged
forketyfork merged 1 commit into
mainfrom
add-comments
Dec 26, 2025
Merged

Document core modules with clarifying comments#3
forketyfork merged 1 commit into
mainfrom
add-comments

Conversation

@forketyfork
Copy link
Copy Markdown
Owner

Summary

  • add intent-level comments to main SDL/ghostty event loop and notification flow
  • document PTY/shell setup and controlling-tty requirements
  • explain font cache behavior and C import facade purpose

Testing

  • not run (comments-only changes)

@forketyfork forketyfork merged commit 6ae585a into main Dec 26, 2025
4 checks passed
@forketyfork forketyfork deleted the add-comments branch December 26, 2025 21:40
forketyfork added a commit that referenced this pull request Jan 5, 2026
This commit addresses all review comments from Copilot and fixes the GitHub Actions build failure:

Build Fix:
- Made cwd.zig macOS-only with comptime check to prevent Linux build failures
- Added conditional import of cwd_mod in state.zig (struct{} for non-macOS)
- Added early return in updateCwd for non-macOS platforms

Review #1 - Ping-pong animation:
- Replaced sawtooth animation with smooth ping-pong pattern
- Animation now includes forward scroll, backward scroll, and idle at both ends
- Eliminates jarring jump back to start

Review #2 - Fade conditions:
- Fixed inverted fade logic
- fade_left now correctly triggers when scroll_offset > 0
- fade_right now correctly triggers when scroll_offset < scroll_range

Review #3 - Documentation:
- Added documentation clarifying cwd_basename is a subslice of cwd_path
- Documents lifetime dependency between the two fields

Review #4 & #5 - Silent failure logging:
- Added log warnings for basename buffer overflow
- Added log warnings for parent path buffer overflow
- Helps debugging when paths exceed buffer limits

Review #6 - Remove unused field:
- Removed cwd_marquee_offset field (animation uses current_time instead)
- Cleaned up unused code

All fixes tested with:
- zig build: Success
- zig build test: All tests pass
- zig fmt src/: Code formatted
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant