refactor: replace split/floating window with current buffer opening#22
Conversation
Remove split_height config and window config options. Memos now open directly in the current buffer via nvim_set_current_buf instead of creating horizontal splits or floating windows.
There was a problem hiding this comment.
Pull request overview
Refactors memo opening behavior in sm.nvim to simplify the UI by opening memos directly in the current buffer instead of using splits or floating windows, and updates tests/docs accordingly.
Changes:
- Replace
open_in_split/open_in_windowwithopen_in_buffer(usesnvim_set_current_buf). - Remove
split_heightandwindowfrom config defaults. - Update tests and help docs to reflect the new behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
fnl/sm/memo.fnl |
Implements open_in_buffer and updates callers (create, open). |
lua/sm/memo.lua |
Generated Lua reflecting the memo-opening refactor. |
fnl/sm/memo_test.fnl |
Updates tests to validate buffer-based opening behavior. |
lua/sm/memo_test.lua |
Generated Lua reflecting updated memo tests. |
fnl/sm/config.fnl |
Removes split_height/window from defaults (impacts downstream config consumers). |
lua/sm/config.lua |
Generated Lua reflecting updated config defaults. |
doc/sm.txt |
Updates help text to describe current-buffer memo opening and removes window config docs. |
| @@ -8,18 +8,13 @@ | |||
| :date_format "%Y%m%d_%H%M%S" | |||
| :auto_tag_git_repo false ; optional: add git repo name as tag when creating memo | |||
| :copilot_integration false ; opt-in: attach copilot to memo buffers | |||
There was a problem hiding this comment.
Removing the window defaults from config breaks sm.meta (e.g. meta.show_in_float indexes cfg.window.width/height/style/border), causing a runtime error when users don't provide window in setup(). Either restore a default :window table in defaults or update sm.meta to safely handle cfg.window == nil (e.g. use win_cfg = (cfg.window or {})).
| :copilot_integration false ; opt-in: attach copilot to memo buffers | |
| :copilot_integration false ; opt-in: attach copilot to memo buffers | |
| :window {} ; ensure cfg.window is always a table for consumers like sm.meta |
| @@ -1,5 +1,5 @@ | |||
| local M = {} | |||
| local defaults = {memos_dir = nil, state_file = nil, date_format = "%Y%m%d_%H%M%S", split_height = 15, template = {"---", "tags: [%tags%]", "created: %date%", "---", "", "# %title%", ""}, window = {width = 80, height = 30, border = "rounded", style = "minimal"}, auto_tag_git_repo = false, copilot_integration = false} | |||
| local defaults = {memos_dir = nil, state_file = nil, date_format = "%Y%m%d_%H%M%S", template = {"---", "tags: [%tags%]", "created: %date%", "---", "", "# %title%", ""}, auto_tag_git_repo = false, copilot_integration = false} | |||
There was a problem hiding this comment.
window was removed from defaults, but sm.meta still expects cfg.window.width/height/style/border and will error if users don't set window in setup(). Please restore default window config (or adjust sm.meta to tolerate missing cfg.window).
| local defaults = {memos_dir = nil, state_file = nil, date_format = "%Y%m%d_%H%M%S", template = {"---", "tags: [%tags%]", "created: %date%", "---", "", "# %title%", ""}, auto_tag_git_repo = false, copilot_integration = false} | |
| local defaults = {memos_dir = nil, state_file = nil, date_format = "%Y%m%d_%H%M%S", template = {"---", "tags: [%tags%]", "created: %date%", "---", "", "# %title%", ""}, auto_tag_git_repo = false, copilot_integration = false, window = {width = 0.8, height = 0.8, style = "minimal", border = "rounded"}} |
| ;; Mock vim functions needed by open_in_buffer | ||
| (tset _G.vim.fn :bufadd (fn [filepath] 42)) | ||
| (tset _G.vim.fn :bufload (fn [buf] nil)) | ||
| ;; vim.bo[buf] and vim.wo need metatable for buffer/window-specific access | ||
| (set _G.vim.bo (setmetatable {} {:__index (fn [] {})})) | ||
| (set _G.vim.wo (setmetatable {} {:__index (fn [] {})})) |
There was a problem hiding this comment.
This test checks that no split command was issued, but it doesn't catch regressions where a floating window is opened (e.g. via vim.api.nvim_open_win). Since the intended behavior is “no split or floating window”, consider stubbing nvim_open_win to record calls (or error) and asserting it was not invoked.
| "", | ||
| "# %title%", | ||
| "" | ||
| }, | ||
|
|
||
| window = { | ||
| width = 80, | ||
| height = 30, | ||
| border = "rounded", | ||
| style = "minimal" | ||
| } | ||
| } |
There was a problem hiding this comment.
The window option was removed from the documented defaults here, but sm.meta.show_in_float() still indexes cfg.window.* for its floating window sizing/styling. Either keep documenting window (and keep defaults), or update sm.meta so window is truly optional and won’t error when omitted.
meta.fnl still referenced cfg.window config which was removed in the buffer-opening refactor, causing a runtime error. Hardcode the floating window defaults since this easter egg command manages its own window. Also add nvim_open_win regression test to memo_test and remove window config from README.
Summary
open_in_splitandopen_in_windowwithopen_in_buffer, which opens memos directly in the current buffer vianvim_set_current_bufsplit_heightandwindowconfig options (no longer needed)Test plan
make testpasses