Allow optional globals be omitted

[bm-w]

When defining an optional global, currently the library still requires that it be set in the ExecutionConfig::globals passed to File::execute, or it fails with an ExecutionError(MissingGlobalVariable("MY_GLOBAL")). This change allows the global to be omitted:

constant MY_GLOBAL?

let globals = Variables::new(); // Empty; `MY_GLOBAL` does not need to be set
let config = ExecutionConfig::new(&functions, &globals);
let graph = file.execute(tree, source, &config, &NoCancellation)?;

[hendrikvanantwerpen]

I don't think there's a need for custom logic here. Setting a default value will di the trick here:

global foo? = #null

I checked and see that this feature is missing from the reference documentation. I'll look into that.

[bm-w]

Actually, that gives me a parsing error:

global foo? = #null

Gives:

Expected '"' at (1, 15)

Or:

ParseError::ExpectedToken("\"", Location { row: 0, column: 14 })

Seems globals’ defaults must be strings?

Edit: seems that way:

tree-sitter-graph/src/parser.rs

Line 326 in 68504d7

default = Some(self.parse_string()?);

[hendrikvanantwerpen]

Okay, I see what's going on. I have an idea for a patch. It's not too involved, so I hope I get that done in the coming days somewhere.

[bm-w]

Note: Was writing this comment while you posted your last, so I didn’t see you had an idea for a patch already — FWIW I’ll leave this here as well.

---

I was looking into the global default parsing, and that could be changed to support literals with a change like this (note the todo!("handle unexpected null") — not sure how to handle that):

tree-sitter-graph/src/parser.rs

Line 326 in 68504d7

default = Some(self.parse_string()?);

-            default = Some(self.parse_string()?);
+            if self.peek()? == '#' {
+                default = Some(match self.parse_literal()? {
+                    ast::Expression::TrueLiteral => Value::Boolean(true),
+                    ast::Expression::FalseLiteral => Value::Boolean(false),
+                    ast::Expression::NullLiteral => {
+                        if quantifier == CaptureQuantifier::ZeroOrOne {
+                            Value::Null
+                        } else {
+                            todo!("handle unexpected null")
+                        }
+                    }
+                    _ => unreachable!(),
+                });
+            } else {
+                default = Some(self.parse_string()?.into());
+            }

Where I also changed ast::Global::default from Option<String> to Option<Value>:

tree-sitter-graph/src/ast.rs

Line 59 in 68504d7

pub default: Option<String>,

/// A global variable
 #[derive(Debug, Eq, PartialEq)]
 pub struct Global {
     /// The name of the global variable
     pub name: Identifier,
     /// The quantifier of the global variable
     pub quantifier: CaptureQuantifier,
     /// Default value
-    pub default: Option<String>,
+    pub default: Option<Value>,
     pub location: Location,
 }

I’d be happy to close this PR and open that one? Although I kind of feel it’d be nice to allow users to omit the #null as well (i.e. I’d say merge this PR and also make the change to allow literals as global defaults).

[bm-w]

Hi, @hendrikvanantwerpen! I just noticed this PR still open — did you manage to make any progress on the idea you last said you had?

[hendrikvanantwerpen]

@bm-w Thanks for reminding me. I didn't (and don't) have time to do the patch I had in might. I think your proposal in #156 (comment) is good.

I would suggest to keep it simple:

• Don't try to infer defaults (like #null for an optional), always require it to be set explicitly.
• Accept any value, so if the user sets #null use it. Using the suffixes (like ? or *) to check if #null is valid is quite weak anyway, so it's not going to be 100% safe whatever we do.