feat: split_height設定オプションを追加#21
Merged
Merged
Conversation
メモを水平分割ウィンドウで開く際の高さを設定可能に。 デフォルト値は15行。
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new split_height configuration option that allows users to control the height of the horizontal split window when opening memos. The default value is 15 lines, and setting it to nil will use Neovim's default split behavior.
Changes:
- Added
split_heightconfiguration option with default value of 15 - Modified
open_in_splitfunction to apply the configured height when opening memos - Added test coverage for the new configuration option
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| fnl/sm/config.fnl | Added split_height default configuration value |
| fnl/sm/memo.fnl | Implemented window height setting based on split_height config |
| fnl/sm/memo_test.fnl | Added test to verify split_height functionality |
| lua/sm/config.lua | Auto-generated Lua from Fennel source |
| lua/sm/memo.lua | Auto-generated Lua from Fennel source |
| lua/sm/memo_test.lua | Auto-generated Lua from Fennel source |
Comment on lines
+113
to
+158
| ;; Test split_height config sets window height | ||
| (do | ||
| ;; Clear package.loaded for fresh mocks | ||
| (tset package.loaded :sm.config nil) | ||
| (tset package.loaded :sm.git nil) | ||
| (tset package.loaded :sm.memo nil) | ||
|
|
||
| ;; Track nvim_win_set_height calls | ||
| (var set_height_calls []) | ||
| (when (not _G.vim.api) | ||
| (set _G.vim.api {})) | ||
| (tset _G.vim.api :nvim_win_set_height | ||
| (fn [win height] | ||
| (table.insert set_height_calls {:win win :height height}))) | ||
|
|
||
| ;; Mock config with split_height | ||
| (tset package.loaded :sm.config | ||
| {:get (fn [] | ||
| {:split_height 15 | ||
| :copilot_integration false | ||
| :date_format "%Y%m%d_%H%M%S" | ||
| :template ["---" "# %title%" ""]}) | ||
| :get_memos_dir (fn [] "/tmp/test-memos")}) | ||
|
|
||
| ;; Mock other dependencies | ||
| (tset package.loaded :sm.git {:get_repo_tag (fn [] nil) :is_git_repo (fn [] false)}) | ||
| (tset package.loaded :sm.state {:set_last_edited (fn []) :add_recent (fn []) :load (fn [] {})}) | ||
|
|
||
| ;; Mock vim functions needed by open_in_split | ||
| (tset _G.vim.fn :bufadd (fn [filepath] 1)) | ||
| (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 [] {})})) | ||
| (tset _G.vim :cmd (fn [cmd] nil)) | ||
| (tset _G.vim.api :nvim_win_set_buf (fn [win buf] nil)) | ||
|
|
||
| ;; Re-require memo with mocks | ||
| (local M3 (require :sm.memo)) | ||
|
|
||
| ;; Call open_in_split | ||
| (M3.open_in_split "/tmp/test-memos/test.md") | ||
|
|
||
| ;; Verify nvim_win_set_height was called with correct value | ||
| (assert (= (length set_height_calls) 1) "split_height: nvim_win_set_height called once") | ||
| (assert (= (. set_height_calls 1 :height) 15) "split_height: height set to config value")) |
There was a problem hiding this comment.
The test only covers the case where split_height is set to 15. According to the PR description, setting split_height to nil should skip the height setting and use Neovim's default behavior. A test case should be added to verify that nvim_win_set_height is NOT called when split_height is nil.
| :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 | ||
| :split_height 15 ; height for horizontal split window |
There was a problem hiding this comment.
The new split_height configuration option is not documented in README.md. The configuration table in the Options section (around line 119-127) should include an entry for split_height explaining its purpose, default value (15), and that it can be set to nil to use Neovim's default behavior.
Suggested change
| :split_height 15 ; height for horizontal split window | |
| :split_height 15 ; height for horizontal split window (set to nil to use Neovim's default behavior) |
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.
Summary
split_heightオプションで設定可能にnilを設定した場合は高さ設定をスキップ（Neovimのデフォルト動作）Usage
Test plan
make test- 全テスト通過🤖 Generated with Claude Code