fix(footnotes): add reference only when inserting existing footnote ID#137
fix(footnotes): add reference only when inserting existing footnote ID#137YousefHadder merged 1 commit intomainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes the footnote insertion behavior to allow adding references to existing footnote definitions, rather than rejecting the insertion with a warning. This enables users to insert multiple references to the same footnote, which is a common markdown pattern.
Key changes:
- Removed the early-return check that prevented inserting references to existing footnotes
- Reference is now inserted unconditionally
- When a definition already exists, the cursor is positioned after the reference and an informative notification is shown
- When a definition doesn't exist, the original behavior of creating both reference and definition is preserved
| local new_line = line:sub(1, col + 1) .. reference .. line:sub(col + 2) | ||
| vim.api.nvim_buf_set_lines(bufnr, row - 1, row, false, { new_line }) | ||
|
|
||
| -- If definition already exists, just insert the reference and notify |
There was a problem hiding this comment.
[nitpick] The comment "just insert the reference and notify" is misleading because the reference has already been inserted at lines 123-127. This block only handles the early return case for existing definitions. Consider rewording to: "If definition already exists, position cursor and notify (reference already inserted above)"
| -- If definition already exists, just insert the reference and notify | |
| -- If definition already exists, position cursor and notify (reference already inserted above) |
| -- If definition already exists, just insert the reference and notify | ||
| if existing_def then | ||
| -- Move cursor after the inserted reference | ||
| vim.api.nvim_win_set_cursor(0, { row, col + 1 + #reference }) | ||
| utils.notify( | ||
| "Added reference to existing footnote [^" .. id .. "] (definition at line " .. existing_def.line_num .. ")", | ||
| vim.log.levels.INFO | ||
| ) | ||
| return | ||
| end |
There was a problem hiding this comment.
The new behavior of allowing references to existing footnote definitions lacks test coverage. Since the repository has comprehensive test coverage for other footnote functionality (parser, navigation, etc. in spec/markdown-plus/footnotes_spec.lua), this new code path should also be tested. Consider adding tests that verify:
- Inserting a reference to an existing footnote definition
- The notification message content and log level
- Cursor positioning after insertion
| -- If definition already exists, just insert the reference and notify | ||
| if existing_def then | ||
| -- Move cursor after the inserted reference | ||
| vim.api.nvim_win_set_cursor(0, { row, col + 1 + #reference }) | ||
| utils.notify( | ||
| "Added reference to existing footnote [^" .. id .. "] (definition at line " .. existing_def.line_num .. ")", | ||
| vim.log.levels.INFO | ||
| ) | ||
| return | ||
| end |
There was a problem hiding this comment.
The documentation states that insert_footnote() "Creates both reference and definition" (see doc/markdown-plus.txt line 1822-1823), but the new behavior allows inserting only a reference when the definition already exists. This is a behavior change that should be documented. Consider updating:
- The function's docstring/description in
doc/markdown-plus.txt - The usage instructions in README.md (line 254 currently says "create new footnote references and definitions")
to clarify that this function can either create a new footnote OR add a reference to an existing one.
When inserting a footnote with an existing ID, add reference only since definition exists.