Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix typo in accessing BlockEntryToken member #139

Merged
merged 2 commits into from
May 24, 2024

Conversation

tanmaykm
Copy link
Contributor

Corrected a typo in accessing the end_mark field from span of a BlockEntryToken. Discovered issue while parsing a specification file that resulted in:

julia> github_spec = YAML.load_file("api.git.luolix.top.yaml")
ERROR: type BlockEntryToken has no field end_mark
Stacktrace:
  [1] getproperty
    @ ./Base.jl:37 [inlined]
  [2] parse_indentless_sequence_entry(stream::YAML.EventStream)
    @ YAML ~/.julia/packages/YAML/U6OOW/src/parser.jl:415
  [3] peek(stream::YAML.EventStream)
    @ YAML ~/.julia/packages/YAML/U6OOW/src/parser.jl:54
  [4] _compose_sequence_node(start_event::YAML.SequenceStartEvent, composer::YAML.Composer, anchor::Nothing)
    @ YAML ~/.julia/packages/YAML/U6OOW/src/composer.jl:137
  [5] compose_sequence_node(composer::YAML.Composer, anchor::Nothing)
...

Corrected a typo in accessing the `end_mark` field from `span` of a `BlockEntryToken`.
Discovered issue while parsing a specification file that resulted in:

```
julia> github_spec = YAML.load_file("api.git.luolix.top.yaml")
ERROR: type BlockEntryToken has no field end_mark
Stacktrace:
  [1] getproperty
    @ ./Base.jl:37 [inlined]
  [2] parse_indentless_sequence_entry(stream::YAML.EventStream)
    @ YAML ~/.julia/packages/YAML/U6OOW/src/parser.jl:415
  [3] peek(stream::YAML.EventStream)
    @ YAML ~/.julia/packages/YAML/U6OOW/src/parser.jl:54
  [4] _compose_sequence_node(start_event::YAML.SequenceStartEvent, composer::YAML.Composer, anchor::Nothing)
    @ YAML ~/.julia/packages/YAML/U6OOW/src/composer.jl:137
  [5] compose_sequence_node(composer::YAML.Composer, anchor::Nothing)
...
```
@kescobo
Copy link
Collaborator

kescobo commented May 20, 2024

fantastic, thank you for the contribution! Can you add a test that notes this bug?

I'm a bit concerned that if someone wrote this accessor, it must have worked at some point. We don't really have a settled YAML spec that this package supports (xref #133) - was this changed at some point?

@tanmaykm
Copy link
Contributor Author

This does look like a bug to me. I have added a MWE and test for the fix.

For comparison, this is how the python YAML parser parses the file that fails in Julia without this fix:

Python 3.10.12 (main, Nov 20 2023, 15:14:05) [GCC 11.4.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import yaml
>>> with open('empty_list_elem.data', 'r') as stream:
...     loaded = yaml.safe_load(stream)
... 
>>> print(loaded)
{'testlist': [None, 'one', 'two']}

With this fix, Julia parses it to a comparable:

Dict{Any,Any}("testlist" => Union{Nothing, String}[nothing, "one", "two"])

@kescobo kescobo added the bug label May 21, 2024
@kescobo
Copy link
Collaborator

kescobo commented May 21, 2024

OK, I think I'm happy to merge as a bug fix and then revert if someone complains 😅

I'm just going to throw a link in the #data slack for a couple of days to be sure there's nothing obvious. Please ping again if I haven't moved on this by friday

@tanmaykm
Copy link
Contributor Author

@kescobo hope there have been no objections and we can have this merged?

@kescobo kescobo merged commit 87b88ff into JuliaData:master May 24, 2024
20 checks passed
@kescobo
Copy link
Collaborator

kescobo commented May 24, 2024

Thanks for the bump! JuliaRegistries/General#107585

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants