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

Only rename identifiers that are textually equal #491

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

Conversation

sorawee
Copy link
Contributor

@sorawee sorawee commented Sep 6, 2020

It is possible to have an arrow between two identifiers that are not
textually equivalent (definitely possible in DrRacket, and theoretically
possible in Racket Mode).

In the above event, renaming one identifier should not change the other.

See also racket/drracket#415 for a similar fix in DrRacket, and
racket/racket#3391 which would start to make Racket Mode behave
incorrectly. In particular, with the above PR, renaming
the identifier a in the following program should not rename
all-defined-out as well.

#lang racket
(provide (all-defined-out))
(define a 1)

It is possible to have an arrow between two identifiers that are not
textually equivalent (definitely possible in DrRacket, and theoretically
possible in Racket Mode).

In the above event, renaming one identifier should not change the other.

See also racket/drracket#415 for a similar fix in DrRacket, and
racket/racket#3391 which would start to make Racket Mode behave
incorrectly. In particular, with the above PR, renaming
the identifier `a` in the following program should not rename
`all-defined-out` as well.

    #lang racket
    (provide (all-defined-out))
    (define a 1)
@sorawee
Copy link
Contributor Author

sorawee commented Sep 7, 2020

Note that with the above mentioned PR, attempting to run racket-xp-rename at all-defined-out in

#lang racket
(provide (all-defined-out))
(define a 1)
(define b 1)

will offer users to rename a random defined identifier (it's b for me). While this doesn't seem ideal, the problematic behavior that I attempt to fix with this PR (proceeding to rename it should not rename all-defined-out) is indeed repaired. So I don't consider it to be in the scope of this current PR.

@sorawee sorawee marked this pull request as draft September 7, 2020 20: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.

None yet

1 participant