Code Monkey home page Code Monkey logo

Comments (9)

CyberShadow avatar CyberShadow commented on August 21, 2024

Hi, would you be at all interested in submitting the grammar fix upstream to https://github.com/dlang/dlang.org ? This repository could then pick up the fix.

from tree-sitter-d.

FabArd avatar FabArd commented on August 21, 2024

The problem is not the grammar of the language but the way it has been defined in your grammar.js file.

    // https://dlang.org/spec/statement.html#NonEmptyStatement
    non_empty_statement: $ =>
      choice(
        $._non_empty_statement_no_case_no_default,
        $.case_statement,
        $.case_range_statement,
        $.default_statement,
      ),

I assume that the underscore in "$._non_empty_statement_no_case_no_default," means that the node should not be generated.
Can you allow the generation of this node?
I tried to make the change by recompiling your repository but when I integrate the d.so file in emacs it displays an error message saying that the file is too recent.
Can you tell me how to avoid this version problem so that I can test it directly?

from tree-sitter-d.

CyberShadow avatar CyberShadow commented on August 21, 2024

I assume that the underscore in "$._non_empty_statement_no_case_no_default," means that the node should not be generated.
Can you allow the generation of this node?

Oh, I see what you mean, thanks for clarifying. Tree-sitter calls this "hidden" rules/nodes: https://tree-sitter.github.io/tree-sitter/creating-parsers#hiding-rules

The logic which decides which nodes should be hidden or not is here:

// Choose which definitions should be hidden (inlined) in the tree-sitter AST.
// In the generated grammar, such definitions' names begin with an underscore.
private void scanHidden()
{
foreach (defName, ref def; defs)
{
if (def.kind == Def.Kind.chars)
continue; // Always represents a token; referencees are inlined
// We make a definition hidden if it always contains at most one other definition.
// Definitions which directly contain tokens are never hidden.
// Exception: nodes which contain only one reference and nothing else
// are implicitly understood to have semantic meaning, and are not hidden.
if (def.node.match!(
(ref Reference v) => true,
(ref _) => false,
))
continue;
size_t scanNode(ref Node node)
{
return node.match!(
(ref RegExp v) => unexpected(v).progn(0),
(ref LiteralChars v) => unexpected(v).progn(0),
(ref LiteralToken v) => 2,
(ref Reference v) => 1,
(ref Repeat1 v) => v.nodes.each!scanNode() * 2,
(ref SeqChoice v) => v.nodes.map!(choiceSeq => choiceSeq.map!scanNode().sum()).reduce!max,
(ref _) => unexpected(_).progn(0),
);
}
def.hidden = scanNode(def.node) <= 1;
}
}

Though, I don't understand how unhiding NonEmptyStatementNoCaseNoDefault would help here. Wouldn't you need to only distinguish between NonEmptyStatement and BlockStatement to decide whether to indent it or not?

Also, is there any way for me to try out the indentation rules you're working on, so that I can play around with them and understand the problem for myself better?

I tried to make the change by recompiling your repository but when I integrate the d.so file in emacs it displays an error message saying that the file is too recent.
Can you tell me how to avoid this version problem so that I can test it directly?

I think such an error indicates that the library is built against a different version of tree-sitter. You could try updating this repository (by changing package.json / package-lock.json) to use the version of tree-sitter used by your editor, or change your editor plugin to use the tree-sitter version used by this repository.

from tree-sitter-d.

FabArd avatar FabArd commented on August 21, 2024

Also, is there any way for me to try out the indentation rules you're working on, so that I can play around with them and understand the problem for myself better?

Here are the rules I use to do my indentation tests :

(defcustom tree-sitter-indent-d-tree-sitter-scopes
  '((indent-all . ;; these nodes are always indented
                (
		 ))
    (indent-rest . ;; if parent node is one of this and node is not first → indent
                 (
		  case_statement
		  default_statement
		  ))
    (indent-body . ;; if parent node is one of this and current node is in middle → indent
                 (
		  enum_body
		  aggregate_body ; class body
		  block_statement
		  token_string
		  token_string_token
		  ))
    (paren-indent . ;; if parent node is one of these → indent to paren opener
                  (
		   ))
    (align-char-to . ;; chaining char → node types we move parentwise to find the first chaining char
                   (
		    ))
    (aligned-siblings . ;; siblings (nodes with same parent) should be aligned to the first child
                      (
                       primary_expression
		       postfix_expression
		       ))
    (multi-line-text . ;; if node is one of this, then don't modify the indent
                     ;; this is basically a peaceful way out by saying "this looks like something
                     ;; that cannot be indented using AST, so best I leave it as-is"
                     (
		      ))
    (outdent . ;; these nodes always outdent (1 shift in opposite direction)
             (
	      )))
  "Scopes for indenting in D."
  :type 'sexp
  :group 'd-mode-tree-sitter)

I found a solution to get around this problem but it creates another problem.

Though, I don't understand how unhiding NonEmptyStatementNoCaseNoDefault would help here. Wouldn't you need to only distinguish between NonEmptyStatement and BlockStatement to decide whether to indent it or not?

I suppose that if some nodes are not generated in the tree it is not to overload it with useless nodes.
I think that making the "NonEmptyStatementNoCaseNoDefault" node appear would solve this problem as well as the previous one (issue #10).

SwitchStatement:
    switch ( Expression) [ScopeStatement]

CaseStatement:
    case [ArgumentList] : [ScopeStatementList]

DefaultStatement:
    default : [ScopeStatementList] opt

ScopeStatementList:
    [StatementListNoCaseNoDefault]

StatementListNoCaseNoDefault:
    [StatementNoCaseNoDefault]
    [StatementNoCaseNoDefault] StatementListNoCaseNoDefault

StatementNoCaseNoDefault:
    [EmptyStatement]
    [NonEmptyStatementNoCaseNoDefault]
    [ScopeBlockStatement]

I think such an error indicates that the library is built against a different version of tree-sitter.

I installed the tree-sitter 0.20.7 directly on my system. I wonder if it should not be installed through the package manager of node js ?

from tree-sitter-d.

CyberShadow avatar CyberShadow commented on August 21, 2024

Here are the rules I use to do my indentation tests :

Thanks, I will try it when I get a chance.

I suppose that if some nodes are not generated in the tree it is not to overload it with useless nodes.

Yes, especially things like https://dlang.org/spec/expression.html#AddExpression which might not actually be an "add expression".

I think that making the "NonEmptyStatementNoCaseNoDefault" node appear would solve this problem

I can look into this.

as well as the previous one (issue #10).

I would need to look at #10 again because I understood it as requesting a node for something that might be empty, which is not allowed by tree-sitter.

I installed the tree-sitter 0.20.7 directly on my system. I wonder if it should not be installed through the package manager of node js ?

I think that would depend on how your editor loads it. It probably loads the version from your system.

from tree-sitter-d.

FabArd avatar FabArd commented on August 21, 2024

I installed the tree-sitter 0.20.7 directly on my system. I wonder if it should not be installed through the package manager of node js ?

To get around this problem:

tree-sitter generate --abi 13

Here is the test I did:
Make 'non_empty_statement' appear in the tree only for '_scope_statement'.

++    // this node need to be visible in _scope_statement
++    non_empty_statement: $ =>
++      choice(
++        $._non_empty_statement_no_case_no_default,
++        $.case_statement,
++        $.case_range_statement,
++        $.default_statement,
++      ),

      // https://dlang.org/spec/statement.html#ScopeStatement
      _scope_statement: $ =>
        choice(
--        $._non_empty_statement,
++        $.non_empty_statement,
          $.block_statement,
        ),

For all 'then_satement' and 'else_statement' without block I get the following tree:

     ...
     then_statement:
       non_empty_statement:
         expression_statement:
           expression:
	   ...

I can make a request to select all the 'non_empty_statement' but I can't
indent this node : it doesn't appear in the list of parents.
Do you know why?

from tree-sitter-d.

CyberShadow avatar CyberShadow commented on August 21, 2024

That diff doesn't look right. There is already a _non_empty_statement in the grammar, so you'd want to just rename the definition and references (there's five of them).

from tree-sitter-d.

FabArd avatar FabArd commented on August 21, 2024

So it is impossible to hide nodes in some places and make them appear in others?
The problem is the same if I create a new non_empty_scope_statement.

from tree-sitter-d.

CyberShadow avatar CyberShadow commented on August 21, 2024

In theory you could do:

    non_empty_statement: $ => $._non_empty_statement,

    _non_empty_statement: $ => /* original definition here*/,

    ...

then use $.non_empty_statement when you want it to be visible, and $._non_empty_statement otherwise.

However, for this repository, I think what we want is a consistent set of rules which decide which nodes in the official D grammar should be visible or hidden in the tree-sitter AST. For example, I'm thinking of trying making a node visible if it is referenced by more than one rule.

from tree-sitter-d.

Related Issues (10)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.