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

Add node types #8

Closed
pfroud opened this issue Jun 2, 2022 · 8 comments
Closed

Add node types #8

pfroud opened this issue Jun 2, 2022 · 8 comments

Comments

@pfroud
Copy link
Contributor

pfroud commented Jun 2, 2022

I want to use kaitai_struct_model in my fork of kaitai_struct_gui to make colored icons for different types of tree nodes.

The idea is inspired by IDEs, for example this Java file:

class MyClass {
  int myField;
  void myMethod() {}
  interface MyInterface {}
  enum MyEnum {}
}

Produces these trees in different IDEs:

java navigation trees in different IDEs

Links to what the different icons mean in different IDEs:

To change icons in a JTree it looks like we use a TreeCellRenderer, which is separate from a TreeModel. So I think the icons implementation can't be a part of the kaitai_struct_model project. But I want kaitai_struct_model to distinguish different types of entries so an external TreeCellRenderer can check the node types.

Here's a demonstration of colored icons from my fork:

colored icons in kaitai-struct tree

  • Teal square is a StructNode
  • Red circle is a ListNode
  • Purple triangle is a SimpleNode

Here's the KSY file I am using:

meta:
  id: test
seq:
  - id: sequential_item
    type: u1
  - id: enum_sequential_item
    type: u1
    enum: my_enum
  - id: my_array
    type: my_type
    repeat: expr
    repeat-expr: 2
  - id: the_rest
    size-eos: true
instances:
  my_instance:
    pos: 1
    type: u1
  my_value_instance:
    value: 1
enums:
  my_enum:
    1: first_enum_value
    2: second_enum_value
types:
  my_type:
    seq:
      - id: foo
        type: u1
      - id: bar
        type: u1        

Any suggestions for things to add to the test ksy file? I know it does not include params.

I can think of two dimensions for the icons:

  1. Whether it is a sequential item or an instance
  2. Data type (array, Struct, primitive, enum, ...)

What do you think?

@pfroud pfroud changed the title Add new node types Add node types Jun 2, 2022
@Mingun
Copy link
Owner

Mingun commented Jun 3, 2022

Yeah, I likes the idea. Actually, I created this project for using in the kaitai_struct_gui, but unfortunately Kaitai team are very slow in reaction. Current generated classes has some fundamental restrictions that does not allow to correctly determine their spans in the input in some cases.

But I want kaitai_struct_model to distinguish different types of entries so an external TreeCellRenderer can check the node types.

This already should be possible using instanceof checks -- all node classes are public. I think, that including specific TreeCellRenderer with specific icons should be done in the application projects to keep this project small.

Any suggestions for things to add to the test ksy file? I know it does not include params.

One specific thing that now is not working is a correct handling of repeat-ed nodes. This can be fixed if this set of PRs will be merged.

Parameters could be supported once kaitai-io/kaitai_struct_compiler#191 will be merged.

Whether it is a sequential item or an instance

Yes, it is probably need to add a new property to the ChunkNode. Distinguish node types are not required, because KaitaiStruct explicitly tries to hide any difference between sequential and instance nodes. Something like isSequential() would enough. Together with proposed ValueInstanceNode the class diagram would be:

classDiagram
TreeNode <|.. ValueNode
  ValueNode <|-- ChunkNode
    ChunkNode <|-- SimpleNode
    ChunkNode <|-- StructNode
    ChunkNode <|-- ListNode
  ValueNode <|-- ValueInstanceNode
  ValueNode <|-- ParamNode

class TreeNode {
  <<interface>>
}
class ValueNode {
  <<abstract>>
}
class ChunkNode {
  <<abstract>>
  isSequential(): boolean
}
Loading

Data type (array, Struct, primitive, enum, ...)

That should be already possible. Feel free to fill an issue or send a PR if you found something that is not enough.

@pfroud
Copy link
Contributor Author

pfroud commented Jun 3, 2022

I see you did a lot of work two years ago on both the KaitaiStruct compiler and runtime, but nothing has been merged, that is frustrating!

You are right, I used instanceof to distinguish the existing Node types. I thought we would need to create classes for EnumNode, PrimitiveNode, etc. But that is not needed, I can just do instanceof checks on getValue().

I will keep in mind the bug about repeat-ed nodes. It appears the web IDE handles it correctly.

In this issue kaitai-io/kaitai_struct_java_runtime#26 you included screenshots of kaitai_struct_model integrated into kaitai_struct_gui:

Can you push that code to your fork of kaitai_struct_gui? I want to use it :)

Whether it is a sequential item or an instance

I didn't know GitHub can render class diagrams, that is cool! Here's a link for reference... https://mermaid-js.github.io/mermaid/#/./classDiagram

Are there any other possibilities for "location in KSY file" aside from field and instance? If no, adding isSequential() or isInstance() should work.

If value instances and parameters are the only things that do not have length, then the proposed class diagram looks good to me.

@Mingun
Copy link
Owner

Mingun commented Jun 3, 2022

Can you push that code to your fork of kaitai_struct_gui? I want to use it :)

Sure: https://github.com/Mingun/kaitai_struct_gui/tree/struct-model

Are there any other possibilities for "location in KSY file" aside from field and instance?

Only that two

@pfroud
Copy link
Contributor Author

pfroud commented Jun 3, 2022

Can you push that code to your fork of kaitai_struct_gui? I want to use it :)

Sure: https://github.com/Mingun/kaitai_struct_gui/tree/struct-model

Thanks, I only checked the master branch of your fork!

Data type (array, Struct, primitive, enum, ...)

That should be already possible. Feel free to fill an issue or send a PR if you found something that is not enough.

I think it doesn't work if the SimpleNode value is null. I will submit a small pull request (#9)

@pfroud
Copy link
Contributor Author

pfroud commented Jun 7, 2022

I think this issue can be closed

@pfroud pfroud closed this as completed Jun 7, 2022
@pfroud
Copy link
Contributor Author

pfroud commented Jun 21, 2022

Hi again, do you have code available to add tree nodes for Params, Fields, and Instances?

(The struct-model branch of your fork of kaitai_struct_gui does not have it)

image

@Mingun
Copy link
Owner

Mingun commented Jun 21, 2022

Those nodes was removed in 1451bbd because I found that this is too inconvenient when you need to constantly expand deep structures. I do not recommend use such grouping, it is better to use different styles to differentiate them

@pfroud
Copy link
Contributor Author

pfroud commented Jun 21, 2022

OK, good to know, thank you

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

No branches or pull requests

2 participants