fktn-k / fkYAML

A C++ header-only YAML library
MIT License
67 stars 7 forks source link

handle line advancement when parsing AnchorPrefix and TagPrefix #368

Closed alienczf closed 1 month ago

alienczf commented 2 months ago

Description

when parsing anchor_prefix and tag prefix, the code currently doesn't advance line if we happend to consume newline character.

this results in the subsequent tokens being labelled with wrong line number

            case lexical_token_t::ANCHOR_PREFIX:
            case lexical_token_t::TAG_PREFIX:
                deserialize_node_properties(lexer, type, line, indent);

                // Skip updating the current indent to avoid stacking a wrong indentation.
                //
                //   &foo bar: baz
                //   ^
                //   the correct indent width for the "bar" node key.

+               line = lexer.get_lines_processed();
                continue;

Reproduction steps

sample.yaml

values:
- &id070 !XXX
  source: !YYY
    name: fv
  include: false
#include "fkYAML/node.hpp"
int main(int argc, char* argv[]) {
  std::ifstream ifs("/tmp/sample.yaml");
  fkyaml::node root = fkyaml::node::deserialize(ifs);
  return 0;
}

Expected vs. actual results

wrongly throws indentation error

Minimal code example

No response

Error messages

No response

Compiler and operating system

clang17 ubuntu22

Library version

develop

Validation

fktn-k commented 2 months ago

@alienczf
Thanks for filing the issue.
I'll take a look later.
The fix will be available hopefully this weekend (I'll be unavailable this weekdays).

alienczf commented 2 months ago

all good. though i've decided to give up on yaml in cpp and have some external script to do a conversion into json first. upon further digging, it seems like for really large yaml files: anchors and tags are not fully preserved. I've done some debugging locally and noticed that when the element is first parsed the fields are attached correctly, but when the final document is returned the attributes go missing. I've not done a full trace because the document is a bit too large to trace easily and i'm not familiar enough with this codebase to set the right conditional breakpoints

On Tue, Jul 16, 2024 at 8:31 PM fktn @.***> wrote:

@alienczf https://github.com/alienczf Thanks for filing the issue. I'll take a look later. The fix will be available hopefully this weekend (I'll be unavailable this weekdays).

— Reply to this email directly, view it on GitHub https://github.com/fktn-k/fkYAML/issues/368#issuecomment-2230771995, or unsubscribe https://github.com/notifications/unsubscribe-auth/AECBFFUSQLWEHFHJOS3V3TLZMUHDRAVCNFSM6AAAAABK56LMDCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEMZQG43TCOJZGU . You are receiving this because you were mentioned.Message ID: @.***>

fktn-k commented 1 month ago

@alienczf
Sorry for replying too late.
I've finally got enough time to investigate the root cause of the reported bug and fixed it with some related bugs which were also found during the investigation (PR #369).
The fixed version of this library is now available in the develop branch.

alienczf commented 1 month ago

No worries, thanks for the update!

On Sat, 17 Aug 2024, 11:51 fktn, @.***> wrote:

@alienczf https://github.com/alienczf Sorry for replying too late. I've finally got enough time to investigate the root cause of the reported bug and fixed it with some related bugs which were also found during the investigation (PR #369 https://github.com/fktn-k/fkYAML/pull/369). The fixed version of this library is now available in the develop branch.

— Reply to this email directly, view it on GitHub https://github.com/fktn-k/fkYAML/issues/368#issuecomment-2294611028, or unsubscribe https://github.com/notifications/unsubscribe-auth/AECBFFR5ZU7WUFRQOGLHLS3ZR3CCVAVCNFSM6AAAAABK56LMDCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDEOJUGYYTCMBSHA . You are receiving this because you were mentioned.Message ID: @.***>

fktn-k commented 1 month ago

I'll close this issue since the reported bug has been fixed.
Please reopen this issue if there's something related to this issue but still unresolved.