- Notifications
You must be signed in to change notification settings - Fork61
Fix renaming of labelled arguments#872
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| // ^ren yy | ||
| letfoo2= (~xx)=>xx+1 | ||
| // ^ren yy |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Sofoo was working andfoo2 wasn't?
Not sure I get what's going on.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
They both work, but when forfoo the tilde is dropped from~xx so it becomes(yy) => ..., and forfoo2 the tilde was added to the body(~yy) => ~yy + 1.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Perhaps hacking renaming itself to special-case tilde does not break other things.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That was my first idea but it might be a little bit of extra work, because we'll need to read the text from the loc and have a look at that. There's no information carried on that can help us when producing the rename instructions. Might be the best idea anyway though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
That was my first idea but it might be a little bit of extra work, because we'll need to read the text from the loc and have a look at that. There's no information carried on that can help us when producing the rename instructions. Might be the best idea anyway though.
Not sure it makes sense, but perhaps when the length does not match. E.g. if the loc says 3 and it's supposed to be 2. Though that's risky.
This is an attempt to fix renaming of labelled arguments breaking because it doesn't account for
~(so~is either removed from the argument definition, or added to local bindings that are renamed).This seems to have broken a few other things though that I'm a bit unsure of how to fix. Maybe this isn't the optimal approach to this problem anyway.