Merge dev into main - #3
Merged
Merged
Conversation
…ain for consistency
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the test suite and build system:
- Removes old E2E test infrastructure and unit tests (test_version.lua, test_constants.lua)
- Adds focused integration tests for FFI and Git boundaries (18 tests total)
- Implements CMake-based build with auto-generated standalone scripts (build.sh, build.cmd)
- Bundles utf8proc dependency to eliminate external dependencies
- Adds version management system with semantic versioning support
- Extracts command logic into separate commands.lua module
Reviewed Changes
Copilot reviewed 40 out of 45 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_ffi_integration.lua | New FFI boundary integration tests (10 tests) |
| tests/test_git_integration.lua | New Git operations integration tests (8 tests) |
| tests/run_tests.sh | Simplified test runner for integration tests |
| tests/README.md | Updated documentation explaining new test focus |
| lua/vscode-diff/commands.lua | Extracted command implementations from plugin entry |
| plugin/vscode-diff.lua | Simplified to use commands module |
| c-diff-core/CMakeLists.txt | Complete CMake build system with script generation |
| build.sh / build.cmd | Auto-generated standalone build scripts |
| scripts/bump_version.mjs | Version bumping utility |
| c-diff-core/vendor/* | Bundled utf8proc library files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
esmuellert
added a commit
that referenced
this pull request
May 29, 2026
…files (#401) (#402) ## Summary Fixes #401 — inline layout flickered and reset the cursor when viewing staged newly-added files. Side-by-side was unaffected. ## Root cause `inline_view.show_single_file` allocated a fresh scratch buffer via `nvim_create_buf(false, true)` on every call for virtual files (revision-based content). Each git fs_event refresh tick (~500ms) re-invoked it, swapped a new bufnr into the modified window, and reset the cursor. The side-by-side equivalent (`load_virtual_file` in `side_by_side.lua:790`) already used `bufadd(virtual_file.create_url(...))` — which returns a stable bufnr keyed by `(git_root, revision, path)`. Inline simply missed the trick. This PR brings inline in line with the existing convention. ## Why only staged newly-added inline showed it | Path | Bufnr stability | |---|---| | Inline + virtual revision (`A`/`D` with rev) | **broken** — `nvim_create_buf` churns | | Inline + real file (`??`, unstaged) | OK — `bufadd(file_path)` reuses | | Side-by-side + any single-file virtual | OK — `load_virtual_file` already uses `bufadd(create_url)` | | Tracked-file diff (status `M`) | OK — `view.update` pins buffers | Status `??` in inline didn't flicker because it hit the real-file branch. `D` inline had the same latent flicker but rarely surfaced (viewing a deleted file while git is changing). Fixed as a side effect. ## Empirical confirmation Probe driving the explorer with simulated refresh ticks, observing `session.modified_bufnr` and cursor line: **Before:** ``` INITIAL: bufnr=10 cursor.line=5 REFRESH #1: bufnr=10 cursor.line=5 REFRESH #2: bufnr=12 cursor.line=1 ← new buf, cursor reset REFRESH #3: bufnr=12 cursor.line=1 REFRESH #4: bufnr=14 cursor.line=1 REFRESH #5: bufnr=16 cursor.line=1 ``` **After:** ``` INITIAL: bufnr=6 cursor.line=5 REFRESH #1: bufnr=6 cursor.line=5 REFRESH #2: bufnr=6 cursor.line=5 REFRESH #3: bufnr=6 cursor.line=5 REFRESH #4: bufnr=6 cursor.line=5 REFRESH #5: bufnr=6 cursor.line=5 ``` ## Changes ### `lua/codediff/ui/view/inline_view.lua` Replace the `nvim_create_buf` virtual-file branch with `bufadd(virtual_file.create_url(opts.git_root, opts.revision, opts.rel_path or file_path))` + `bufload`. Mirrors `side_by_side.load_virtual_file` exactly. `BufReadCmd` in `core/virtual_file.lua` handles content loading and intentionally avoids setting `filetype` to prevent LSP attach crashes on the `codediff://` URI scheme. ### `tests/ui/view/inline_explorer_spec.lua` +2 tests: - Test 5: Repeated `show_single_file` for a real file keeps `modified_bufnr` stable. - Test 6: Repeated `show_single_file` for a staged virtual file (`:0`) keeps `modified_bufnr` stable. Direct regression test for #401. ## Testing All 19 regression-relevant spec files pass (172 tests, 0 failures), including: - `inline_explorer_spec.lua` (6) — includes new tests - `inline_standalone_spec.lua` (14) - `inline_history_spec.lua` (4) - `inline_interaction_spec.lua` (12) - `layout_toggle_spec.lua` (13) - `conflict_dedup_spec.lua` (4) - `view_spec.lua` (14) - `explorer_staging_spec.lua` (3) - `virtual_file_lsp_spec.lua` (2) - + others ## Benefits - Stops user-visible flicker and cursor reset reported in #401 - Mirrors the existing `side_by_side.load_virtual_file` convention — no new patterns introduced - Minimal diff (~9 LoC change + tests) — easy to review - No public API changes - Fixes the latent same-class flicker in deleted-file inline view ## Follow-up (not in this PR) `on_file_select` still has no dedup for the status `A`/`??`/`D` early-return branches in either layout. With this fix, that's a wasted-CPU issue (calls to `inline.clear`, `auto_refresh.disable`, `lifecycle.update_*`, `keymaps.setup_all_keymaps`, `layout.arrange` on every ~500ms tick) but no longer a visible bug, since bufnr is now stable. The recurring "missing dedup branch" bug class (#308 staged-rename, #320 conflict, #401 staged-add) suggests `on_file_select` should be refactored to a `resolve_view → render` pipeline with a single dedup site shared across layouts. Worth a follow-up issue to capture the technical debt.
Copilot AI
added a commit
that referenced
this pull request
Jul 29, 2026
…lows Addresses code scanning alerts #1-#5 (actions/missing-workflow-permissions). Adds `permissions: contents: read` at workflow level to: - .github/workflows/build-and-test.yml (alert #1) - .github/workflows/pr.yml (alerts #2, #3, #4) - .github/workflows/regression-check.yml (alert #5) Co-authored-by: yanuoma <220247883+yanuoma@users.noreply.github.com>
esmuellert
pushed a commit
that referenced
this pull request
Jul 29, 2026
…lows Addresses code scanning alerts #1-#5 (actions/missing-workflow-permissions). Adds `permissions: contents: read` at workflow level to: - .github/workflows/build-and-test.yml (alert #1) - .github/workflows/pr.yml (alerts #2, #3, #4) - .github/workflows/regression-check.yml (alert #5) Co-authored-by: yanuoma <220247883+yanuoma@users.noreply.github.com>
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
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.
No description provided.