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 traverse-with-path #1300

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

shirok
Copy link
Collaborator

@shirok shirok commented Oct 10, 2024

It is a variation of traverse, except the actions receives a path-thunk argument after the visiting node. When called within the action, path-thunk returns a list of ascendant nodes of the visiting node, i.e. its car is the immediate parent, and its the last element is the root node on which traverse-with-path is called.

This is useful for AST walker that needs wider context surrounding the node of concern.

This PR supersedes #1291. This one avoid relying on special variables.

Copy link
Collaborator

@amorphedstar amorphedstar left a comment

Choose a reason for hiding this comment

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

This looks pretty good. I wonder if there is a more elegant way to handle multiple wrapping actions. The current traversal code treats an action-list almost like an alist so that defaults can be overridden, but this PR makes it clear that it could also be nice to use an action-list like an actual sequence of commands.

Comment on lines 278 to 283
(let ((r (apply (action-function action)
node
path-thunk
args)))
(pop traversal-path)
r)))))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you could also just prog1 this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

right.

(make-action when 'node
(lambda (node &rest _rest)
(declare (ignore rest))
(funcall op node))))))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Wait, what is op? Is this supposed to be a copy of the ecase above, just with the (apply (action-function action) node path-thunk args) removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good catch. Remnant of the previous version.

It is a variation of `traverse`, except the actions receives a `path-thunk`
argument after the visiting node.  When called within the action, `path-thunk`
returns a list of ascendant nodes of the visiting node, i.e. its car is
the immediate parent, and its the last element is the root node on which
`traverse-with-path` is called.

This is useful for AST walker that needs wider context surrounding
the node of concern.
@shirok shirok force-pushed the traverse-with-path branch from 730198c to 5e19de9 Compare October 10, 2024 08:25
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