Skip to content

Conversation

patdhlk
Copy link
Collaborator

@patdhlk patdhlk commented Sep 24, 2025

  • allow source trace comments in YAML
  • add TCs for YAML
  • add documentation for YAML as supported comment type
  • add documentation for C-Sharp as supported comment type
  • YAML support #36

- allow source trace comments in YAML
- add TCs for YAML
- add documentation for YAML as supported comment type
- add documentation for C-Sharp as supported comment type
- #36
@patdhlk patdhlk requested a review from juiwenchen September 24, 2025 06:39
Tests for:

- Basic key-value pairs
- Nested structures
- List items
- Document structures with ---
- Flow mapping structures
- complex structures and configs
Copy link
Contributor

@juiwenchen juiwenchen left a comment

Choose a reason for hiding this comment

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

Nice work!

It would be nice to know how inline comment will behave in yaml. From the current implementation, it seems it will always find the next node_block as the associated scope. For example

key1: value1  # inline comment
key2: value2

The associated block of the comment is key2:value. I also checked how this works in python, and it faces the same situation unless there is no next_named_sblings.

I would propose two options:

  • Suggest not to use inline comments in our documentation if info of their associated scope are required
  • In our implementation, we can look for prev_named_sibling on the same row of the comment before go to next_named_sibling.

The PR is already good to merge, and we can just create a GitHub Issue for the record if we eventually want to support inline comments.

Up to you

- consider YAML inline comments, e.g. key1:value1 #comment1
- add respective TCs
@patdhlk
Copy link
Collaborator Author

patdhlk commented Sep 24, 2025

@juiwenchen thanks for the hint, I updated the respective YAML parts to support also inline comments.

@patdhlk patdhlk requested a review from juiwenchen September 24, 2025 10:03
Copy link
Contributor

@juiwenchen juiwenchen left a comment

Choose a reason for hiding this comment

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

Awesome!

Let's update change_log.rst in the next PR.

@patdhlk patdhlk merged commit 3a22828 into main Sep 24, 2025
8 checks passed
@patdhlk patdhlk deleted the 36-yaml-support branch September 24, 2025 11:24
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.

2 participants