Skip to content

fix: pass opts.name to serpent to correct reference deserialize#2672

Merged
phanen merged 1 commit intomainfrom
fix-serialize
Apr 18, 2026
Merged

fix: pass opts.name to serpent to correct reference deserialize#2672
phanen merged 1 commit intomainfrom
fix-serialize

Conversation

@phanen
Copy link
Copy Markdown
Collaborator

@phanen phanen commented Apr 16, 2026

Before, obj/func ref is sometimes deserialized as nil since:

  1. serpent don't know who fn1 before the full table is returned since
    the serialized data is a compact version to ensure func/obj ref is
    consistant
  2. lua table is random, so it's also possible to deserialize fn2
    correctly, but fn1=nil
return { fn1 = my_func, fn2 = fn1 }

Apply opts.name="_" for serpent, it would be like:

  local _ = { fn1 = my_func }
  _.fn2 = _.fn1
  return _

Summary by CodeRabbit

  • Refactor

    • Improved serialization and execution wiring to streamline how previews and dynamic code are prepared and run; highlight parsing now uses an inspected/stringified format for clearer output.
  • Tests

    • Added coverage to verify serialization preserves shared function references and updated headless tests to use the revised execution approach.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 16, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 812c65f6-e92a-40bc-8fcb-4be0a0c5b6a0

📥 Commits

Reviewing files that changed from the base of the PR and between be70ce8 and 675295d.

📒 Files selected for processing (6)
  • lua/fzf-lua/libuv.lua
  • lua/fzf-lua/previewer/builtin.lua
  • lua/fzf-lua/test/exec_lua.lua
  • lua/fzf-lua/test/helpers.lua
  • tests/headless_spec.lua
  • tests/libuv_spec.lua
✅ Files skipped from review due to trivial changes (1)
  • tests/libuv_spec.lua
🚧 Files skipped from review as they are similar to previous changes (1)
  • lua/fzf-lua/previewer/builtin.lua

📝 Walkthrough

Walkthrough

Updates to serialization and runtime wiring: serpent calls now include name = "_" in two modules, previewer highlight serialization switched from serpent to vim.inspect, exec path refactored to accept an exec_lua function, tests adjusted to run jobs directly and a new round-trip test for shared function references added.

Changes

Cohort / File(s) Summary
LibUV serialization change
lua/fzf-lua/libuv.lua
M.serialize now passes name = "_" to serpent.line, altering the produced Lua source string metadata.
Test exec_lua refactor
lua/fzf-lua/test/exec_lua.lua, lua/fzf-lua/test/helpers.lua
M.run signature changed to accept an exec_lua function; serpent.block call updated with name = "_"; helpers now pass child_lua into exec_lua.run.
Previewer highlight formatting
lua/fzf-lua/previewer/builtin.lua
Previewer.highlights:parse_entry replaced serpent-based serialization with vim.inspect(hl_def, { indent = (" "):rep(fn.shiftwidth()) }) and adjusted line handling.
Headless test adjustments
tests/headless_spec.lua
Removed serpent dependency in exec_term, now executes jobstart/jobwait via in-Nvim c.lua(...) wrapper; changed new_child.start()new_child.init() in server test.
New libuv round-trip test
tests/libuv_spec.lua
Added test asserting serialize/deserialize preserves shared function references (fn1 and fn2 point to same function and return expected result).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • ibhagwan

Poem

🐰 A tiny tweak, a subtle art,
serpent trimmed, inspect takes part.
Functions link and tests run through,
shared references stay true.
Hoppity hop — code says woohoo! 🌿

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: passing opts.name to serpent to fix deserialization of shared references.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix-serialize

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@tests/libuv_spec.lua`:
- Around line 202-212: The test currently only checks behavior of deserialized
functions; update the "serialize/deserialize with shared function reference"
test to also assert identity preservation by adding an assertion that
deserialized.fn1 == deserialized.fn2 after deserialization (retain checks using
libuv.serialize and libuv.deserialize and existing type/return-value assertions
for fn1 and fn2).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 5ffd6eec-3724-410e-b8ca-ccdfb22fb956

📥 Commits

Reviewing files that changed from the base of the PR and between bd281b9 and be70ce8.

📒 Files selected for processing (5)
  • lua/fzf-lua/libuv.lua
  • lua/fzf-lua/previewer/builtin.lua
  • lua/fzf-lua/test/exec_lua.lua
  • tests/headless_spec.lua
  • tests/libuv_spec.lua

Comment thread tests/libuv_spec.lua
Before, obj/func ref is sometimes deserialized as nil since:
1. serpent don't know who fn1 before the full table is returned since
   the serialized data is a compact version to ensure func/obj ref is
   consistant
2. lua table is random, so it's also possible to deserialize fn2
   correctly, but fn1=nil
```
return { fn1 = my_func, fn2 = fn1 }
```

Apply opts.name="_" for serpent, it would be like:
```
  local _ = { fn1 = my_func }
  _.fn2 = _.fn1
  return _
```

ci: pass exec_lua function

ci: refactor test
@phanen phanen merged commit 9363b23 into main Apr 18, 2026
11 checks passed
@phanen phanen deleted the fix-serialize branch April 18, 2026 09:17
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