joelspadin / tree-sitter-devicetree

Tree-sitter grammar for Devicetree files
MIT License
28 stars 5 forks source link

Define labeled items recursively #4

Closed nickcoutsos closed 1 year ago

nickcoutsos commented 2 years ago

Hi again!

I'm at a point where I need to parse all of the labels for a node from the devicetree source but currently the additional node yields an error.

Examples

> parser.parse('foo: bar {};').rootNode.toString()
'(document (labeled_item label: (identifier) item: (node name: (identifier))))'
> parser.parse('foo: bar: baz {};').rootNode.toString()
'(document (labeled_item label: (identifier) (ERROR (identifier)) item: (node name: (identifier))))'

I'm sure my solution isn't suitable so this is partly a question of the intended difference between labeled_node and labeled_item and how the aliasing should work. Originally the only way I got the recursion working for arbitrarily many labels was to delete the labeled_node rule and make define labeled_item recursively.

Alternative approach:

Locally I've just tried another change where both rules are recursive versions of themselves:


labeled_node: ($) =>
    seq(
        field('label', $.label_identifier),
        ':',
        field('item', choice($.labeled_node, $.node))
    ),

labeled_item: ($) =>
    seq(
        field('label', $.label_identifier),
        ':',
        field('item', choice($.labeled_item, $.node, $.property))
    ),

This doesn't cause errors, but the results for the same test committed in this branch differ:

    (document
      (labeled_item
        label: (identifier)
        item: (labeled_node
          label: (identifier)
          item: (labeled_node
            label: (identifier)
            item: (labeled_node
              label: (identifier)
              item: (node
                name: (identifier)))))))

The three nested labels are picked up as labeled_node this time instead of labeled_item like the outermost. Is this because the aliasing of the rule only applies to top level items? I'm not sure what you would consider to be more appropriate.

nickcoutsos commented 1 year ago

@joelspadin I'm doing some more work with managing/editing labels and realized it makes more sense to treat multi-labeled nodes as a single labeled_item node with multiple label fields.

I've made a separate branch to show the relevant changes here: https://github.com/joelspadin/tree-sitter-devicetree/compare/main...nickcoutsos:tree-sitter-devicetree:feature/multi-labeled-nodes-alternate

joelspadin commented 1 year ago

Thanks!

nickcoutsos commented 1 year ago

Hi Joel!

Did you have any thoughts on the two approaches? I feel like multiple label fields in a single labeled_item node makes more sense, and it's the approach I'm using now, but I neglected to update this PR to propose that instead.

joelspadin commented 1 year ago

Oh, true. I do think that might make more sense.