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 section "rename symbol" #5

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

Conversation

bendtherules
Copy link
Contributor

@bendtherules bendtherules commented Jan 4, 2020

Part of #2.

Only example added for discussion.
DONT MERGE now.

@bendtherules bendtherules mentioned this pull request Jan 4, 2020
10 tasks
@itsdouges
Copy link
Owner

itsdouges commented Jan 4, 2020

hey well this definitely accomplishes what we wanted! what's your thoughts on the PR in its current shape? (other than there isn't content in the handbook yet)

i like that its a good time to teach why you'd want to use forEachChild. i dislike that it's a lot of code though.

@bendtherules
Copy link
Contributor Author

bendtherules commented Jan 5, 2020

Need to remove some console.logs.

Its basically doing three things -

  1. find the symbol given identifier name and identifier index

  2. Find all identifiers from that symbol

  3. Replace those identifiers with identifier with different name.

Now steps 1 and 2 is more complicated than is required for a naive example. Need step 1 so that you simulate "selecting" a specific repetition of identifier name. So for ex, we have 4 identifiers named foo. Selecting repitition index 0,1,3 renames outer foo, selecting index 2 renames inner foo. So, this also shows how rename is safe even with variable shadowing.

Now, because in step 1 we are selecting a specific repetition (say index 1), we need to start over again (as we have crossed 0) to find all the identifiers related to that symbol. This is step 2.

If we assume that we want to only select index 0,then step 1 and 2 can be merged together. Also, it takes care of not finding any identifier with that name. So, really depends on what we want to show - a basic example (which is smaller, but ignores some cases, good for reading) vs a more robust example (better for copy paste).

@bendtherules
Copy link
Contributor Author

Yes, i also never used forEachChild before.

But its great when you want to just visit all nodes and not really return/modify anything. Also, you can cancel visiting at any point

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.

None yet

2 participants