This is based on guoweis-work PR https://github.com/kovetskiy/mark/pull/145
* feat(confluence): add support for fetching page body and inline comments
* feat(cmd): add --preserve-comments flag to preserve inline comments
* feat(mark): implement context-aware inline comment preservation
* test(mark): add tests for context-aware MergeComments logic
* fix: remove empty else branch in MergeComments to fix SA9003
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* perf: compile markerRegex once as package-level variable
Avoids recompiling the inline comment marker regex on every call to
MergeComments, which matters for pages with many comment markers.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: guard against nil comments pointer in MergeComments
Prevents a panic when GetInlineComments returns nil (e.g. on pages
where the inline comments feature is not enabled).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* test: add edge-case tests for MergeComments; fix overlapping replacement
Four new test cases:
- SelectionMissing: comment dropped gracefully when text is gone from new body
- OverlappingSelections: overlapping comments no longer corrupt the body;
the later match (by position) wins and the earlier overlapping one is dropped
- NilComments: nil pointer returns new body unchanged
- HTMLEntities: <, >, ' selections match correctly
Also fixes the overlapping replacement bug: apply back-to-front and skip any
replacement whose end exceeds the start of an already-applied one.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: escape ref attribute value in inline comment marker XML
Use html.EscapeString on r.ref before interpolating it into the
ac:ref attribute to prevent malformed XML if the value ever contains
quotes or other special characters.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: use first occurrence when no context is available in MergeComments
Without context the old code left distance=0 for every match and
updated bestStart on each iteration, so the final result depended on
whichever occurrence was visited last (non-deterministic with respect
to the search order).
Restructure the loop to break immediately on the first match when
hasCtx is false, making the behaviour explicit and deterministic.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: log warning when overlapping inline comment marker is dropped
Previously the overlap was silently skipped. Now a zerolog Warn message
is emitted with the ref, the conflicting byte offsets, and the ref of
the already-placed marker, so users can see which comment was lost
rather than silently getting incomplete output.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: warn when inline comments are silently dropped in MergeComments
Three cases now emit a zerolog Warn instead of silently discarding:
1. Comment location != "inline": logs ref and actual location.
2. Selected text not found in new body: logs ref and selection text.
3. Overlapping replacement (existing): adds selection text to the
already-present overlap warning for easier diagnosis.
Also adds a selection field to the replacement struct so the overlap
warning can report the dropped text.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: update markerRegex to match markers with nested tags
Replace ([^<]*) with (?s)(.*?) so the pattern:
- Matches marker content that contains nested inline tags (e.g. <strong>)
- Matches across newlines ((?s) / DOTALL mode)
The old character class [^<]* stopped at the first < inside the
marker body, causing the context-extraction step to miss any comment
whose original selection spanned formatted text.
Add TestMergeComments_NestedTags to cover this path.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: guard against empty OriginalSelection in MergeComments
strings.Index(s, "") always returns 0, so an empty escapedSelection
would spin the search loop indefinitely (or panic when currentPos
advances past len(newBody)).
Skip comments with an empty selection early, emit a Warn log, and
add TestMergeComments_EmptySelection to cover the path.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: paginate GetInlineComments to avoid silently truncating results
The Confluence child/comment endpoint is paginated. The previous
single-request implementation silently dropped any comments beyond
the server's default page size.
Changes:
- Add Links (context, next) to InlineComments struct so the _links
field from each page response is decoded.
- Rewrite GetInlineComments to loop with limit/start parameters
(pageSize=100), accumulating all results, following the same pattern
used by GetAttachments and label fetching.
- Add TestMergeComments_DuplicateMarkerRef to cover the deduplication
guard added in the previous commit.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix UTF-8 safety, API compat, log verbosity
- levenshteinDistance: convert to []rune before empty-string checks so
rune counts (not byte counts) are returned for strings with multi-byte
characters
- Add contextBefore/contextAfter helpers that use utf8.RuneStart to
avoid slicing in the middle of a multi-byte UTF-8 sequence when
extracting 100-char context windows from oldBody and newBody
- Add truncateSelection helper (50 runes + ellipsis) and apply it in all
Warn log messages that include the selected text, preventing large or
sensitive page content from appearing in logs
- Downgrade non-inline comment log from Warn to Debug with message
'comment ignored during inline marker merge: not an inline comment';
page-level comments are not inline markers and are not 'lost'
- Restore original one-argument GetPageByID (expand='ancestors,version')
and add GetPageByIDExpanded for the one caller that needs a custom
expand value, preserving backward compatibility for API consumers
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Address new PR review comments
- Remove custom min() function: shadows the Go 1.21+ built-in min for
the entire package; the built-in handles the 3-arg call in
levenshteinDistance identically
- Validate rune boundaries on strings.Index candidates: skip any match
where start or end falls in the middle of a multi-byte UTF-8 rune
to prevent corrupt UTF-8 output
- Defer preserve-comments API calls until after shouldUpdatePage is
determined: avoids unnecessary GetPageByIDExpanded + GetInlineComments
round-trips on no-op --changes-only runs
- Capitalize Usage string for --preserve-comments flag (util/flags.go)
and matching README.md entry to match sentence case of surrounding flags
- Run gofmt on util/cli.go to fix struct literal field alignment
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* docs: document --preserve-comments feature in README
Add a dedicated 'Preserving Inline Comments' section under Tricks with:
- Usage examples (CLI flag and env var)
- Step-by-step explanation of the Levenshtein-based relocation algorithm
- Limitations (deleted text, overlapping selections, new pages,
changes-only interaction)
Also add a cross-reference NOTE near the --preserve-comments flag entry
in the Usage section.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* docs: fix markdownlint errors in README
- Change unordered list markers from dashes to asterisks (MD004)
- Remove extra blank line before Issues section (MD012)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Extract named types for InlineComments; optimize Levenshtein search
- Introduce InlineCommentProperties, InlineCommentExtensions, and
InlineCommentResult named types in confluence/api.go, replacing the
anonymous nested struct in InlineComments.Results. Callers and tests
can now construct/inspect comment objects without repeating the JSON
shape.
- Simplify makeComments helper in mark_test.go to use the new named
types directly, eliminating the verbose anonymous struct literal.
- Add two Levenshtein candidate-search optimisations in MergeComments:
* Exact-context fast path: if both the before and after windows match
exactly, take that occurrence immediately without computing distance.
* Lower-bound pruning: skip the full O(m*n) Levenshtein computation
for a candidate when the absolute difference in window lengths alone
already meets or exceeds the current best distance.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Use stable sort with ref tie-breaker; fix README overlap description
- Replace slices.SortFunc with slices.SortStableFunc for the
replacements slice, adding ref as a lexicographic tie-breaker when
two markers resolve to the same start offset. This makes overlap
resolution fully deterministic across runs.
- Correct the README limitation note: the *earlier* overlapping
match (lower byte offset) is what gets dropped; the later one
(higher byte offset, applied first in the back-to-front pass) is
kept. The previous wording said 'the second one is dropped' which
was ambiguous and inaccurate.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix rune-based lower-bound pruning; clarify test comment
- Use utf8.RuneCountInString instead of len() for the Levenshtein
lower-bound pruning computation. The levenshteinDistance function
operates on rune slices, so byte-length differences can exceed the
true rune-length difference for multibyte UTF-8 content, causing
valid candidates to be incorrectly skipped.
- Update TestMergeComments_SelectionMissing comment to say the comment
is 'dropped with a warning' rather than 'silently dropped', matching
the actual behavior.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Add missing unit tests for helpers and MergeComments scenarios
Helper function tests:
- TestTruncateSelection: short/exact/long strings and multibyte runes
- TestLevenshteinDistance: empty strings, identical, insertions,
deletions, substitutions, 'kitten/sitting', and a multibyte UTF-8
case to exercise rune-based counting
- TestContextBefore / TestContextAfter: basic windowing, window larger
than string, and a case where the raw byte offset lands mid-rune (é)
to verify the rune-boundary correction logic
MergeComments scenario tests:
- TestMergeComments_MultipleComments: two non-overlapping comments both
correctly applied via back-to-front replacement
- TestMergeComments_EmptyResults: non-nil InlineComments with zero
results returns body unchanged
- TestMergeComments_NonInlineLocation: page-level comments (location
!= 'inline') are skipped; body unchanged
- TestMergeComments_NoContext: when a ref has no marker in oldBody the
first occurrence of the selection in newBody is used
- TestMergeComments_UTF8: multibyte (Japanese) characters in both body
and selection are handled correctly
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix three correctness issues in MergeComments
- Fix html import shadowing: alias the 'html' import as 'stdhtml' to
avoid shadowing by the local 'html' variable used throughout
ProcessFile. Both callers updated: stdhtml.EscapeString for the
ref attribute, htmlEscapeText for the selection search.
- Fix selection search with quotes/apostrophes: replace
html.EscapeString for the selection with a new htmlEscapeText helper
that only escapes &, <, > — not ' or ". Confluence storage HTML
often leaves quotes and apostrophes unescaped in text nodes, so
fully-escaped selections would fail to match and inline comments
would be silently dropped. Add TestMergeComments_SelectionWithQuotes.
- Fix duplicate-ref warnings: move seenRefs[ref]=true to immediately
after the duplicate-check, before the search loop. Previously seenRefs
was only set on a successful match, so multiple results for the same
MarkerRef with no match in the new body would each emit a 'dropped'
warning. Add TestMergeComments_DuplicateMarkerRefDropped.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Optimize levenshteinDistance to use two rolling rows instead of full matrix
Reduces memory allocation from O(m×n) to O(n) by keeping only the
previous and current rows. Also swaps r1/r2 so the shorter string is
used for column width, minimizing row allocation size.
This matters in MergeComments where levenshteinDistance is called for
every candidate match of every comment's selection in newBody — on
pages with many comments or short/common selections the number of
calls can be high.
Addresses thread [40] from PR review.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix test description and README algorithm doc
mark_test.go (thread [43]):
- TestMergeComments_HTMLEntities: the description incorrectly claimed
' (apostrophe) was tested; the selection '<world>' contains no
apostrophe. Updated comment to accurately describe what is covered
(</> entity matching) and note the ' limitation.
- Add TestMergeComments_ApostropheSelection: verifies a selection with
a literal apostrophe is found when the new body also has a literal
apostrophe (the common case from mark's renderer). This exercises
the htmlEscapeText path which intentionally does not encode ' or ".
README.md (thread [42]):
- Step 2 of the algorithm description said context was recorded
'immediately before and after the commented selection' which is
ambiguous. Clarified that context windows are taken around the
<ac:inline-comment-marker> tag boundaries in the old body (not
around the raw selection text), so the context is stable even when
the marker wraps additional inline markup such as <strong>.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Unexport mergeComments and cap candidate evaluation
Thread [44]: MergeComments was exported but is internal-only — only
called within the mark package and tested from the same package.
Unexport it to mergeComments to avoid expanding the public API surface
unnecessarily. Add a Go doc comment describing the function contract,
HTML expectations, and the candidate cap.
Thread [45]: The candidate-scoring loop had no upper bound. For short
or common selections (e.g. 'a', 'the') on large pages the loop could
invoke levenshteinDistance thousands of times, each allocating rune
and int slices. Add a maxCandidates=100 constant and break once that
many on-rune-boundary occurrences have been evaluated. The exact-context
fast-path and lower-bound pruning already skip many candidates before
Levenshtein is called, so in practice the cap is only reached for very
common selections where the 100th candidate is unlikely to be
meaningfully better than an earlier one anyway.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* test: fix HTMLEntities description and add ApostropheEncoded limitation test
Thread #43: TestMergeComments_HTMLEntities had a misleading note claiming it
covered the ' apostrophe case, but the selection under test ('<world>') did
not include an apostrophe. Remove that note and add a dedicated
TestMergeComments_ApostropheEncoded test that explicitly documents the known
limitation: when a Confluence body stores an apostrophe as the numeric entity
', mergeComments cannot locate the selection (htmlEscapeText does not
encode ' to '), so the comment is dropped with a warning.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* Fix CDATA selection fallback and extract contextWindowBytes constant
Thread #46: mergeComments only searched for htmlEscapeText(selection) and
would fail for selections inside CDATA-backed macro bodies (e.g. ac:code),
where < and > are stored as raw characters rather than HTML entities. Restructure
the search loop to build a searchForms slice: the escaped form is tried first
(covers normal XML text nodes), and the raw unescaped form is appended as a
fallback when they differ. A stopSearch flag exits early on an exact context
match or when maxCandidates is reached, preserving the same performance
guarantees as before. Add TestMergeComments_CDATASelection to cover this path.
Thread #47: The context-window size 100 was repeated in four places across
mergeComments (two in the context-extraction loop and two in the scoring loop).
Extract it to const contextWindowBytes = 100 so it is easy to tune and stays
consistent everywhere.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace interface{} with any (Go 1.18) across confluence/api.go,
macro/macro.go, util/cli.go, util/error_handler.go, includes/templates.go
- Replace sort.SliceStable with slices.SortStableFunc + cmp.Compare (Go 1.21)
in attachment/attachment.go, consistent with existing slices usage
- Replace fmt.Errorf("%s", msg) with errors.New(msg) in mark.go
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update Go module path from github.com/kovetskiy/mark to
github.com/kovetskiy/mark/v16 across all packages and imports,
following Go module versioning conventions for major versions >= 2.
Also update README installation instructions and version string.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When --continue-on-error was set and one or more files failed to
process, Run() logged each failure but returned nil, making it
impossible for callers or CI systems to detect partial failures.
Track whether any file failed with a hasErrors flag and return a
descriptive error after all files have been attempted.
Update TestContinueOnError to reflect the corrected behaviour: the
test now asserts that an error IS returned (partial failure is
surfaced) while still verifying that all files in the batch are
attempted (not just the first one).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The newLabels parameter was accepted but never used in the function
body; labels are synced through the separate updateLabels/AddPageLabels
/DeletePageLabel calls. The dead parameter misled callers into thinking
labels were being set during the page update.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Markdown conversion failures called panic(err), crashing the process
rather than allowing graceful error handling. Change the return type
to (string, []attachment.Attachment, error) and propagate the error.
Update all callers (mark.go, markdown_test.go) accordingly.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
confluence.NewAPI trims trailing slashes from the base URL into
api.BaseURL. Using config.BaseURL directly could produce double
slashes in the logged/printed URL when the caller passes a
trailing-slash BaseURL.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
errcheck lint requires all error return values to be handled.
Propagate write errors from both Fprintln call sites.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
page.ResolvePage requires non-nil metadata and would error immediately
when called with meta == nil (e.g. when --page-id is used and the file
has no metadata header, or when metadata is intentionally suppressed).
Guard the call: when meta != nil use ResolvePage as before; when meta
is nil but PageID is provided, validate the page exists via
api.GetPageByID instead; when neither is set the earlier mandatory-
field check already returns an error, so no further action is needed.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
mark.Run and ProcessFile were writing directly to os.Stdout via
fmt.Println, which is a surprising side-effect for library callers.
Add Config.Output io.Writer for callers to provide their own sink.
When nil the helper falls back to io.Discard, so library embedders
that do not set Output receive no implicit stdout writes. The CLI
layer sets Output: os.Stdout to preserve existing behaviour.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When PageID mode is used (meta == nil), labels is nil and calling
updateLabels unconditionally treats that as an empty desired set,
silently removing all existing global labels from the page. Guard
the call so label syncing only runs when metadata is present.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Expose the core mark functionality as an importable Go library.
Library users can now import github.com/kovetskiy/mark and call:
err := mark.Run(mark.Config{
BaseURL: "https://confluence.example.com",
Username: "user",
Password: "token",
Files: "docs/**/*.md",
Features: []string{"mermaid", "mention"},
})
The new package provides:
- Config struct: all options decoupled from the CLI framework
- Run(config Config) error: process all files matching Config.Files
- ProcessFile(file, api, config): process a single markdown file
Also moves the CLI entry point to cmd/mark/main.go following standard
Go convention for projects that serve as both a library and a binary.
Fixes a pre-existing nil-pointer dereference on meta.Attachments,
meta.Layout and related fields when using --target-url with a pageId
(meta was nil in that code path).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
