Skip to content

Conversation

@Paalon
Copy link
Contributor

@Paalon Paalon commented Jun 13, 2024

Abstract

  • This is no breaking PR.
  • This adds new features.
  • This is internal change.
  • This is required to develop other features.

Detail

Rewrite construct functions in src/constructor.jl. Add constructor functions for the three essential YAML schemata. However this is not completed because processing for tag !!merge and !!value are still tightly connected to construct_mapping etc.

  • Organize constructors of ConstructorError.
  • Add TODO comments.
  • Change function flatten_mapping to always return nothing.
  • The following are the list of renamed and relocated functions from src/constructor.jl to src/constructor_yaml_jl_0_4_10.jl for the YAML.jl 0.4.10 schema:
    • construct_yaml_nullconstruct_yaml_jl_0_4_10_schema_null
    • bool_valuesyaml_jl_0_4_10_schema_bool_values
    • construct_yaml_boolconstruct_yaml_jl_0_4_10_schema_bool
    • construct_yaml_intconstruct_yaml_jl_0_4_10_schema_int
    • construct_yaml_floatconstruct_yaml_jl_0_4_10_schema_float
    • timestamp_patyaml_jl_0_4_10_schema_timestamp_regex
    • construct_yaml_timestampconstruct_yaml_jl_0_4_10_schema_timestamp
    • construct_yaml_omapconstruct_yaml_jl_0_4_10_schema_timestamp
    • construct_yaml_pairsconstruct_yaml_jl_0_4_10_schema_timestamp
    • construct_yaml_setconstruct_yaml_jl_0_4_10_schema_set
    • construct_yaml_strconstruct_yaml_jl_0_4_10_schema_str
    • construct_yaml_seqconstruct_yaml_jl_0_4_10_schema_seq
    • construct_yaml_mapconstruct_yaml_jl_0_4_10_schema_map
    • construct_yaml_objectconstruct_yaml_jl_0_4_10_schema_object
    • construct_yaml_binaryconstruct_yaml_jl_0_4_10_schema_binary
    • construct_undefinedconstruct_undefined_yaml_jl_0_4_10_schema: this requires test's modification.
    • default_yaml_constructorsyaml_jl_0_4_10_schema_constructors: this requires test's modification.
  • Similar functions are defined for the failsafe, the JSON and the Core schemata in src/constructor_failsafe.jl, src/constructor_json.jl and src/constructor_core.jl.
  • Add tests for the four schemata.

This PR does not change any commonly used API (load, load_all, write, etc.).

Paalon added 10 commits June 13, 2024 19:18
Define constructors for each schema. However, it is not completed. So
this is the part 1.
Because the two tags: `!!merge` and `!!value` remain
in the construction of mapping.
I added a bit modification in the constructor of `ConstructorError`
to increase the readability.
And also I added some TODO comments and my thoughts.
@Paalon Paalon changed the title Rewrite constructor part 1 [Internal, New Feature, No Breaking] Rewrite constructor part 1 Jun 16, 2024
@kescobo
Copy link
Member

kescobo commented Jun 25, 2024

Is this still live?


Constructor(::Nothing) = Constructor(Dict{String,Function}())
SafeConstructor(constructors::Dict = Dict(), multi_constructors::Dict = Dict()) = Constructor(merge(copy(default_yaml_constructors), constructors), multi_constructors)
# Paalon: I don't know what is safe.
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe unsafe refers to things like this that you can do in pyyaml:

$ python
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
>>> yaml.load("""!!python/object/apply:subprocess.Popen\n- [echo, hello]\n""", Loader=yaml.Loader)
<Popen: returncode: None args: ['echo', 'hello']>
>>> hello

@kescobo kescobo added the internal Doesn't implicate user-facing functions label Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Doesn't implicate user-facing functions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants