Fixdecision insertion inside html comment#17
Closed
doguhanniltextra wants to merge 2 commits intoActiveMemory:mainfrom
Closed
Fixdecision insertion inside html comment#17doguhanniltextra wants to merge 2 commits intoActiveMemory:mainfrom
doguhanniltextra wants to merge 2 commits intoActiveMemory:mainfrom
Conversation
On a fresh DECISIONS.md the template contains a multi-line <!-- DECISION FORMATS ... --> comment that includes an example heading '## [YYYY-MM-DD] Decision Title'. Two bugs caused new decisions to land inside that comment block, making them invisible when rendered: 1. insertDecision used strings.Index to find the first '## [' occurrence, which matched the template example inside the comment. 2. insertAfterHeader (the fallback path) only skipped ctx-specific markers (<!-- ctx:context -->, <!-- ctx:end -->), so it stopped at <!-- INDEX:START --> and inserted before the index table and format-guide comment. Fix: - Add isInsideHTMLComment(content, idx) helper that checks whether a byte position falls between a <!-- and its closing -->. - Rewrite insertDecision and insertLearning to walk all '## [' occurrences in a loop, skipping any that isInsideHTMLComment identifies as being inside a comment block. - Update insertAfterHeader to skip any <!-- ... --> block (not just ctx-specific markers), so the fallback path clears the INDEX markers and the DECISION FORMATS guide before placing the entry. Add TestInsertDecisionNotInsideComment regression test that reproduces the exact fresh-file structure from the bug report and asserts the new entry appears after the closing --> of the DECISION FORMATS block. Signed-off-by: doguhanniltextra <doguhangithub@gmail.com>
- Split TestInsertDecisionNotInsideComment into sub-tests:
* first decision lands after comment block
* second decision prepends before first (verifies prepend order
still works correctly after the fix)
- Add TestIsInsideHTMLComment with 8 table-driven cases covering:
* position before, inside, and after a comment
* inline single-line comment (<!-- INDEX:START -->)
* multi-line comment with heading inside vs after close
* unclosed comment (treated as inside)
* content with no comment at all
Signed-off-by: doguhanniltextra <doguhangithub@gmail.com>
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.
Problem
On a fresh
DECISIONS.mdscaffolded byctx init, runningctx add decisioninserts the new entry inside the<!-- DECISION FORMATS ... -->comment block, making it invisible when rendered in any Markdown viewer.Issue
#10
The INDEX table at the top updates correctly, but the decision body is hidden — users don't notice until they view the rendered page.
Root cause
Two bugs in
internal/cli/add/insert.go:insertDecisionusedstrings.Index(content, "## [")which matched the## [YYYY-MM-DD] Decision Titletemplate example that lives inside the<!-- DECISION FORMATS ... -->comment block.insertAfterHeader(the fallback path when no real entries exist yet) only skipped<!-- ctx:context -->and<!-- ctx:end -->markers. It stopped at<!-- INDEX:START -->and inserted before the index table and format-guide comment.Fix
isInsideHTMLComment(content, idx)— new helper that checks whether a byte position falls between a<!--and its closing-->.insertDecision/insertLearning— rewritten to walk all## [occurrences in a loop, skipping any thatisInsideHTMLCommentidentifies as being inside a comment block.insertAfterHeader— updated to skip any<!-- ... -->block (not just ctx-specific markers), so the fallback path clears the INDEX markers and the DECISION FORMATS guide before placing the insertion point.Tests
TestInsertDecisionNotInsideComment— regression test using the exact fresh-file structure from the bug report, with two sub-tests:TestIsInsideHTMLComment— 8 table-driven unit tests for the new helper, covering: before/inside/after a comment, inline single-line comment, multi-line comment with heading inside vs after close, unclosed comment, and no-comment content.All existing tests in
internal/cli/addcontinue to pass.Checklist
go fmt ./internal/cli/add/...— cleango vet ./internal/cli/add/...— cleango test ./internal/cli/add/... -count=1— all passTestInsertDecisionNotInsideComment)TestIsInsideHTMLComment)