Skip to content

Fix missing syntax highlighting in proposed diff view #32

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jun 11, 2025

Conversation

blink-so[bot]
Copy link
Contributor

@blink-so blink-so bot commented Jun 11, 2025

What & Why

When Claude Code shows a Proposed changes diff, the scratch buffer on the right didn’t inherit the original file’s filetype, so Neovim rendered it without syntax highlighting (issue #20).

Key changes in this PR

  1. _detect_filetype helper

    • Tries Neovim’s built-in vim.filetype.match (≥0.10) or falls back to the extension map.
    • Can also read the type from an existing buffer.
  2. Filetype propagation

    • After opening the temp/scratch buffer, the detected filetype is applied with vim.api.nvim_set_option_value.
  3. Tests

    • Added unit spec to assert the filetype is set (using spy.on) so regressions are caught.
  4. Makefile improvements

    • Defines a global NIX_PREFIX that expands to nix develop .#ci -c only when outside a Nix shell, preventing nested shells in CI.
  5. Misc

    • Removed a stray blank line in diff.lua docblock.
    • Consolidated commits into a single, clean history.

Verification

nix fmt                                # no changes
nix develop .#ci -c make check         # luacheck 0 warnings / 0 errors
nix develop .#ci -c make test          # 258 tests, 0 failures

Fixes #20.

Co-authored-by: ThomasK33 [email protected]

@ThomasK33 ThomasK33 changed the title [DRAFT] Fix missing syntax highlighting in proposed diff view (Fixes #20) Fix missing syntax highlighting in proposed diff view Jun 11, 2025
* Adds  helper and propagates filetype to proposed buffers to restore syntax highlighting
* Cleans up duplicate code and stray newline
* Makes  CI-friendly by defining global  to avoid nested nix shells
* Updates unit tests to cover filetype propagation

Co-authored-by: ThomasK33 <[email protected]>
@blink-so blink-so bot force-pushed the fix-proposed-diff-filetype branch from 0ba7d77 to ddad527 Compare June 11, 2025 12:12
@blink-so blink-so bot changed the title Fix missing syntax highlighting in proposed diff view [DRAFT] Fix missing syntax highlighting in proposed diff view (Fixes #20) Jun 11, 2025
@ThomasK33 ThomasK33 changed the title [DRAFT] Fix missing syntax highlighting in proposed diff view (Fixes #20) Fix missing syntax highlighting in proposed diff view Jun 11, 2025
@ThomasK33 ThomasK33 requested a review from Copilot June 11, 2025 12:13
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR ensures that proposed diff buffers inherit the original file’s syntax highlighting by detecting and propagating the filetype, and also refines CI shell invocation.

  • Introduces a _detect_filetype helper for path‐ and buffer‐based filetype resolution.
  • Applies detected filetypes in both native diff and split diff views.
  • Adds a unit test for native-diff filetype propagation and updates the Makefile to prevent nested Nix shells in CI.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
tests/unit/diff_spec.lua Added a test to verify filetype propagation in _open_native_diff.
lua/claudecode/diff.lua Implemented _detect_filetype and applied filetype in diff views.
Makefile Defined NIX_PREFIX to skip nix develop when already in a shell.
Comments suppressed due to low confidence (2)

lua/claudecode/diff.lua:721

  • Add a unit test for _create_diff_view_from_window to verify that the filetype is correctly propagated in split diff views, matching the native-diff test pattern.
-- Ensure new buffer inherits filetype from original for syntax highlighting (#20)

lua/claudecode/diff.lua:722

  • Ensure that original_buffer is defined in this scope before passing it to _detect_filetype. If original_buffer is undefined, this will cause a runtime error.
local original_ft = _detect_filetype(old_file_path, original_buffer)

Comment on lines +248 to +269
local simple_map = {
lua = "lua",
ts = "typescript",
js = "javascript",
jsx = "javascriptreact",
tsx = "typescriptreact",
py = "python",
go = "go",
rs = "rust",
c = "c",
h = "c",
cpp = "cpp",
hpp = "cpp",
md = "markdown",
sh = "sh",
zsh = "zsh",
bash = "bash",
json = "json",
yaml = "yaml",
yml = "yaml",
toml = "toml",
}
Copy link
Preview

Copilot AI Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider extracting the simple_map table to a module-level constant so it isn’t recreated on every call, improving performance and clarity.

Suggested change
local simple_map = {
lua = "lua",
ts = "typescript",
js = "javascript",
jsx = "javascriptreact",
tsx = "typescriptreact",
py = "python",
go = "go",
rs = "rust",
c = "c",
h = "c",
cpp = "cpp",
hpp = "cpp",
md = "markdown",
sh = "sh",
zsh = "zsh",
bash = "bash",
json = "json",
yaml = "yaml",
yml = "yaml",
toml = "toml",
}

Copilot uses AI. Check for mistakes.

diff.setup({})

-- Spy on nvim_set_option_value
spy.on(_G.vim.api, "nvim_set_option_value")
Copy link
Preview

Copilot AI Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] After spying on nvim_set_option_value, consider restoring or removing the spy in an after_each block to avoid side effects on other tests.

Copilot uses AI. Check for mistakes.

@ThomasK33 ThomasK33 enabled auto-merge June 11, 2025 12:22
@ThomasK33 ThomasK33 added this pull request to the merge queue Jun 11, 2025
Merged via the queue into main with commit 09b231d Jun 11, 2025
5 checks passed
@ThomasK33 ThomasK33 deleted the fix-proposed-diff-filetype branch June 11, 2025 12:23
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.

[BUG] Proposed change has no syntax highlighting
1 participant