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

Statements: Parsing & dependency management #1

Merged
merged 12 commits into from
Nov 14, 2023
Merged

Conversation

neenaoffline
Copy link
Member

This PR adds statement parsing & some dependency management. Statement evaluation will be added in a follow-up PR.

Comment on lines 173 to 175
(->> updated-addrs
(deps/immediate-dependents sheet)
(deps/resolve-dependents sheet)))))))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would passing updated-addrs to resolve-dependents directly take care of getting the immediate dependents?

Comment on lines 133 to 136
(defn eval-statements [scratch]
(comment
eval scratch
generate bindings names -> ast))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can remove.

#{})))

(defn resolve-dependents [{:keys [depgraph]} deps]
"Iterate over the dependents and return a set of resolved dependents. :ref
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could break this into two resolve-binding-dependents, then this could just be called dependents.

src/bean/deps.cljs Show resolved Hide resolved
src/bean/ui/scratch.cljs Show resolved Hide resolved
To add content to statemnents, we annotate the AST with the source using
instaparse's built-in functionality.
@@ -0,0 +1,46 @@
(ns bean.ui.scratch
(:require [bean.scratch :as scratch]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename scratch ns to code.

src/bean/deps.cljs Show resolved Hide resolved
(map ast->deps args))
:FunctionDefinition (ast->deps arg)
:Name (if (#{"x" "y" "z"} arg) #{} #{(->named-dep arg)})
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move this check to a formal-parameter? in interpreter.

(let [updated-dependent-set (disj (get depgraph parent) child)]
(if (empty? updated-dependent-set)
(dissoc depgraph parent)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need to dissoc parents with no children?

src/bean/grid.cljs Show resolved Hide resolved
@@ -134,7 +110,7 @@
(spill grid))))

(defn parse-grid [grid]
(util/map-on-matrix content->cell grid))
(util/map-on-matrix value/from-cell grid))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This name seems incorrect, it's more like to-cell. This is the input to the interpreter, the interpreter returns a value. See ast-result.

:representation (str "Undefined reference: \"" name "\"")})

(defn named-ref-error [named error]
(str "name: " named ". " error))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note to self: Look at named-ref-error usage.

[bean.deps :refer [make-depgraph update-depgraph]]
[clojure.test :refer [deftest testing is]]))

(deftest depgraph-update-test
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a test for named deps.

(let [evaled-grid (grid/eval-sheet [["1" "2" "=A1+B1"]
["=C1" "" ""]])]
(let [evaled-grid (grid/eval-sheet
(grid/new-sheet [["1" "2" "=A1+B1"] ["=C1" "" ""]] ""))]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Split grid into two lines if the linter allows. It's easier to read.

Comment on lines +17 to +18
(name (or (get-in @sheet [:ui :scratch-evaluation-state])
:evaluated)))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should put scratch-evaluation-state evaluated by default instead of defaulting here.

{:grid parsed-grid
:bindings {}
:depgraph (make-depgraph parsed-grid)})
(defn new-sheet [content-grid code]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can make code default to "" unless supplied.

eval-sheet
{:grid grid*
:depgraph (cond-> depgraph
content-changed? (update-depgraph
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe can skip content-changed? for simplicity.

@prabhanshuguptagit prabhanshuguptagit merged commit da18f0a into main Nov 14, 2023
1 check passed
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

Successfully merging this pull request may close these issues.

2 participants