-
-
Notifications
You must be signed in to change notification settings - Fork 93
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
Outline for syncheck:add-arrow/name-dup/pxpy/renamable #417
base: master
Are you sure you want to change the base?
Conversation
I think this is reasonable, but I also think that there are few (very few) languages where you can rename identifiers that aren't spelled the same way. I have mixed feelings about backwards incompatibility here, simply because a lot of the current renaming behavior just looks just like a plain bug. We could generalize the The other thing I'm unsure of is where to put this property. The way case-sensitivity works is at the reader, not the expander, so it seems a bit strange for the property to be coming from the macros instead of the reader (somehow). For example, someone might make a meta-language that changes reader parameters and the macros won't know what property to put in. |
Bouncing among the various issues and PRs, I think I'm getting confused what specific bugs we're trying to fix, and/or features to add. Having said that: When I did the rename feature for
And so on...? And if Those would be new features, not bug fixes (IMHO). How desirable they would be, relative to the cost of doing it, I don't know.
|
p.s. I guess the new feature imaginings are all hopelessly |
The That said, the problem here stems from the observation that we don't seem to have the right information in the fully expanded code to know if a binding arrow implies a renaming possibility or not. @sorawee suggested we could look at the buffer and just see if the head and tail of the arrows are ranges in the editor with the same sequences of letters in them. I think this is a great idea but we are currently thwarted by languages that are case-insensitive and the fear of the unknown (specifically that there are languages out there relying on the non-existence of this check that want to let you rename two different names to become the same name ... for some unknown reason). So we are discussing and trying to figure out a way forward with the goal of somehow having the traversals.rkt code provide more information to racket-mode and to DrRacket's GUI about which arrows correspond to a renaming possibility and which ones don't. I hope this makes some sense! |
What backwards incompatibility? I believe my proposal is backwards compatible.
The syntax property
You convinced me in the other discussion to stay away from textual code, so I don't think it's a good idea make case-insensitive comparison the default. To make it backward compatible, the default of Note that renaming only applies when an arrow is drawn. There's no arrow between
But there's an arrow between
In both cases, the default
One use case of
With the PR, there's an arrow from
|
re backwards incompatibility: yes your plan is backwards compatible but maybe some backward incompatibility is acceptable here because we can see this as fixing bugs. That is, I see the basic issue here as a balance between having to support (as yet unknown) languages that want to rename things whose names are different (so far, case-insensitivity is the only real example we know of) and having to go change a lot of macros to insert this new property into them. I hope this helps clarify my earlier comments. |
My very original goal is to draw arrows from definitions to
Renaming
Indeed, Racket Mode doesn't have this problem because as you said, it doesn't allow renaming when So I'm trying to come up with a general mechanism to specify when it's safe to rename.
I think Check Syntax could be improved in this aspect. Check Syntax currently ad-hoc recognizes A more principled solution, I believe, is by attaching syntax property
But by adjusting the expansion of |
Currently there's an arrow from
This would be another place where we should inhibit renaming, probably by adding the syntax property via the reader. Technically it's OK to rename, but because there's no space between EDIT: also, |
check that the symbolic name of a binder and its reference are the same Also, it appears that something changed along the way such that we no longer need the special case that solved issue #110 anymore
@rfindler here's an implementation of what I discussed earlier in #415.
The PR is only for discussion. At the very least it still needs tests and documentation. Not ready to be merged.