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

Parametrized types do not allow spaces after "(" #792

Closed
Mingun opened this issue Aug 1, 2020 · 3 comments · Fixed by kaitai-io/kaitai_struct_compiler#207
Closed

Parametrized types do not allow spaces after "(" #792

Mingun opened this issue Aug 1, 2020 · 3 comments · Fixed by kaitai-io/kaitai_struct_compiler#207

Comments

@Mingun
Copy link

Mingun commented Aug 1, 2020

Following KSY raises error until space between ( and true will be deleted:

meta:
  id: type_references
seq:
  - id: field
    type: 'type::sub( true , 2 )'
types:
  type:
    types:
      sub:
        params:
          - id: par
            type: bool
          - id: par2
            type: u1
        seq: []

I think that is unnecessary restriction and it can be eliminated. The root of the problem is that parametric type parsing is partially done in manually, and partially using expression language. It is much easier to always parse the entire string as an expression, at the same time you can:

  • allow spaces between ::
  • allow spaces in begin and at end of string
  • allow spaces before (

Now corresponding PEG rule for type references is:

parseTypeRef = name ("::" name)* ("(" args _ ")")? EOS;
args = expr (_ "," _ expr)*;
...

But it would be:

parseTypeRef = _ name (_ "::" _ name)* _ ("(" _ args _ ")" _)? EOS;
@GreyCat
Copy link
Member

GreyCat commented Aug 12, 2020

Makes sense, thanks for reporting it! It's not a super high priority, but hopefully we'll get to fix it sometime in the future.

@generalmimon
Copy link
Member

@Mingun Thanks for this research! I've also encountered this problem in my BMP spec on these lines (image/bmp.ksy:148-151):

        type: 'color_table(
            not header.is_core_header,
            header.extends_bitmap_info ? header.bitmap_info_ext.num_colors_used : 0
          )'

This fails in the devel Web IDE with the following exception:

io.kaitai.struct.format.YAMLParseException: /types/bitmap_info/seq/3: unable to find type 'color_table(
not header.is_core_header,
header.extends_bitmap_info ? header.bitmap_info_ext.num_colors_used : 0
)', searching from bmp

However, KSC under JVM (using SnakeYAML library) works fine:

$ ksc --debug -t javascript --verbose file -I . image/bmp.ksy
parsing image\bmp.ksy...
reading image\bmp.ksy...
... compiling it for javascript...
.... writing Bmp.js

It's possible that the yaml.js is not strictly compliant to the YAML spec, so the document is not parsed properly, but that's not the thing I wanted to show. It looks like quoting the string using quotes is the only way to make it compile under JVM, because in fact all other variants of YAML multiline syntax are rejected:

  1. literal style (| - "Keep newlines (literal)" at https://yaml-multiline.info/)

            type: |
              color_table(
                not header.is_core_header,
                header.extends_bitmap_info ? header.bitmap_info_ext.num_colors_used : 0
              )
    $ ksc --debug -t javascript --verbose file -I . image/bmp.ksy
    parsing image\bmp.ksy...
    reading image\bmp.ksy...
    /types/bitmap_info/seq/3: unable to find type 'color_table(
      not header.is_core_header,
      header.extends_bitmap_info ? header.bitmap_info_ext.num_colors_used : 0
    )
    ', searching from bmp
    
  2. folded style with strip chomping indicator (>- - "Replace newlines with spaces (folded)" with "No newline at end (strip)" at https://yaml-multiline.info/)

            type: >-
              color_table(
                not header.is_core_header,
                header.extends_bitmap_info ? header.bitmap_info_ext.num_colors_used : 0
              )
    $ ksc --debug -t javascript --verbose file -I . image/bmp.ksy
    parsing image\bmp.ksy...
    reading image\bmp.ksy...
    /types/bitmap_info/seq/3: unable to find type 'color_table(
      not header.is_core_header,
      header.extends_bitmap_info ? header.bitmap_info_ext.num_colors_used : 0
    )', searching from bmp
    

So I think it totally makes sense to tolerate the whitespace.

@generalmimon
Copy link
Member

BTW, a similar issue has been addressed by kaitai-io/kaitai_struct_compiler#133.

Mingun added a commit to Mingun/kaitai_struct_compiler that referenced this issue Dec 28, 2020
Mingun added a commit to Mingun/kaitai_struct_compiler that referenced this issue Dec 28, 2020
Mingun added a commit to Mingun/kaitai_struct_compiler that referenced this issue May 11, 2021
GreyCat pushed a commit to kaitai-io/kaitai_struct_compiler that referenced this issue Aug 24, 2021
Mingun added a commit to Mingun/ksc-rs that referenced this issue Nov 6, 2021
…nce parser

Fix a bug of not parsed absolute paths of the user types
Mingun added a commit to Mingun/ksc-rs that referenced this issue Jul 13, 2024
- process name can be a path with components delimited by dot
- kaitai-io/kaitai_struct#792 is fixed, so remove comment about disallowed space
- allow space after the process name or the `)` finishing the arguments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants