Skip to content

Introduce explicit top-level multi-embedding configuration#16

Merged
Vladyslav-Kuksiuk merged 9 commits intomasterfrom
update-configuration
May 5, 2026
Merged

Introduce explicit top-level multi-embedding configuration#16
Vladyslav-Kuksiuk merged 9 commits intomasterfrom
update-configuration

Conversation

@Vladyslav-Kuksiuk
Copy link
Copy Markdown
Collaborator

This PR updates how multiple embedding configurations are set. Resolves this issue.

Changes:

  • Replaced the ambiguous embed-mappings structure with explicit top-level embeddings.
  • Made each embedding a complete, named configuration with unique names and isolated fragment output under fragments-path/<embedding-name>.
  • Improved validation and CLI output for multi-embedding configs, including warnings for shared docs-path values.

@Vladyslav-Kuksiuk Vladyslav-Kuksiuk self-assigned this Apr 30, 2026
@Vladyslav-Kuksiuk Vladyslav-Kuksiuk marked this pull request as ready for review April 30, 2026 14:56
Copy link
Copy Markdown
Collaborator

@dmytro-kashcheiev dmytro-kashcheiev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Vladyslav-Kuksiuk LGTM with two minor things to check before the merge.

Comment thread README.md
Comment on lines +149 to +152
* `embeddings`: (Optional) A list of complete embedding configurations for multiple
documentation targets. When `embeddings` is set, do not set root-level `code-path`
or `docs-path`. Define `code-path`, `docs-path`, and optional settings inside each entry.
Each entry must set a unique `name`.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If both options set, it should throw with meaningfull message to make configuration issues clear.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does.

image

Comment thread cli/cli_validation.go Outdated
Comment on lines +167 to +168
// Validates one embedding entry.
func validateEmbeddingConfig(embedding EmbeddingConfig, index int) error {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure if this and the above doc style match the GO best practices.
Below, you start with the function name. Did I miss something?

@alexander-yevsyukov
Copy link
Copy Markdown
Contributor

@Vladyslav-Kuksiuk, please update the title of the PR with precise description of the feature. "Update configuration behaviour" is too generic.

@Vladyslav-Kuksiuk Vladyslav-Kuksiuk changed the title Update configuration behaviour Introduce explicit top-level multi-embedding configuration May 5, 2026
# Conflicts:
#	bin/embed-code-linux
#	bin/embed-code-macos
#	bin/embed-code-windows.exe
@Vladyslav-Kuksiuk Vladyslav-Kuksiuk merged commit 2e91559 into master May 5, 2026
3 checks passed
@Vladyslav-Kuksiuk Vladyslav-Kuksiuk deleted the update-configuration branch May 5, 2026 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve handling of the multiple docs targets

3 participants