feat: Add #insert directive for simple content inclusion#42
Draft
agonzalezrh wants to merge 8 commits intomasterfrom
Draft
feat: Add #insert directive for simple content inclusion#42agonzalezrh wants to merge 8 commits intomasterfrom
agonzalezrh wants to merge 8 commits intomasterfrom
Conversation
- Add new #insert directive that includes content without merge strategies - Implement parseInsert function to parse #insert directives - Add getAllInserts function to collect insert directives recursively - Update merge logic to handle #insert directives with selective merging - For __meta__ section, only merge sandboxes and deployer fields - For non-__meta__ fields, use simple overwrite - Process #include directives within inserted files - Add test case for parseInclude with options This provides a cleaner alternative to complex merge strategies when you want to simply add content without applying custom merge logic.
- Remove TestParseIncludeWithOptions and TestIncludeHasOption tests for notmerge - Add TestParseInsert test for new #insert directive functionality - Clean up test file to focus on current #insert implementation
- Preserve critical fields: asset_uuid, components, catalog, anarchy, ansible_control_plane, ansible_controller_select_mode, ansible_controllers - Allow merging of sandboxes and deployer fields from insert - Special handling for deployer section to merge sub-fields (like execution_environment) - Maintains local values while adding content from inserted files This ensures that #insert adds content without overwriting critical local configuration.
- Only add fields that don't already exist in local configuration - Preserve all local variables and __meta__ fields - Add new content from insert without overwriting existing values - Much simpler and cleaner implementation This ensures #insert truly just adds content without any overwrites.
- Remove all complex preservation logic - Just add all content from insert files - Simple and straightforward approach - Processes includes within inserted files correctly
- Just include the content of the insert file - No complex merging or preservation logic - Simple and straightforward approach - Works correctly without schema validation errors
- Process includes within inserted files correctly - Preserve local variables like asset_uuid and components - Only add new __meta__ fields that don't already exist - Add non-__meta__ fields only if they don't exist - Tested and working correctly
- Removes unused function that was causing linter error - Clean up code
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
This PR adds a new directive that provides a clean way to include content without applying merge strategies.
Problem
Users needed a way to include content from other YAML files without the complex merge strategies that applies. The existing directive applies all merge strategies, which can be too aggressive for simple content inclusion.
Solution
Added a new directive that:
Changes
Usage
Benefits
Testing