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

make-visible not finding unsaved-editor #707

Closed
shhyou opened this issue Jan 4, 2025 · 5 comments · Fixed by #708
Closed

make-visible not finding unsaved-editor #707

shhyou opened this issue Jan 4, 2025 · 5 comments · Fixed by #708

Comments

@shhyou
Copy link
Collaborator

shhyou commented Jan 4, 2025

Version: 8.15.0.7 [cs]

Related to: d27a480

Given this program in an unsaved editor:

#lang htdp/bsl


(check-expect 1 "no")

The source link at line 4, column 0 in the output does not highlight check-expect and does not bring up the current tab, unlike the case for a saved editor:

Welcome to DrRacket, version 8.15.0.7 [cs].
Language: htdp/bsl, with debugging; memory limit: 256 MB.
Ran 1 test.
0 tests passed.

Check failures:
        Actual value 1 differs from "no", the expected value.
at line 4, column 0
> 

It seems like this change to find-matching-tab works around the issue, but I don't know what's the right fix:

        (define/public (find-matching-tab filename)
          (define fn-path (if (string? filename)
                              (string->path filename)
                              filename))
          (for/or ([tab (in-list tabs)])
-           (define tab-filename (send (send tab get-defs) get-filename))
-           (and tab-filename
-                (pathname-equal? fn-path tab-filename)
-                tab)))
+           (cond
+             [(symbol? fn-path)
+              (and (or (send (send tab get-defs) port-name-matches? fn-path)
+                       (send (send tab get-ints) port-name-matches? fn-path))
+                   tab)]
+             [else
+              (define tab-filename (send (send tab get-defs) get-filename))
+              (and tab-filename
+                   (pathname-equal? fn-path tab-filename)
+                   tab)])))

from:

(define/public (find-matching-tab filename)
(define fn-path (if (string? filename)
(string->path filename)
filename))
(for/or ([tab (in-list tabs)])
(define tab-filename (send (send tab get-defs) get-filename))
(and tab-filename
(pathname-equal? fn-path tab-filename)
tab)))

@rfindler
Copy link
Member

rfindler commented Jan 4, 2025

That kind of change is the right kind but it doesn't need the symbol? guard (you always just prefer port-name-matches? when it likes something) but I'm not sure it is the right change because I'm not sure how we're getting to find-matching-tab to know if find-matching-tab is supposed to accept only paths or supposed to also accept the names from ports.

@rfindler
Copy link
Member

rfindler commented Jan 4, 2025

I see that make-visible, which calls find-matching-tab is already doing the port-matching thing, so that suggests that maybe moving something else might be a better solution.

@rfindler
Copy link
Member

rfindler commented Jan 4, 2025

Yeah (and sorry for the multiple comments! I'll stop after this one to wait for a reply), it looks like this commit probably should have been done differently.

@shhyou
Copy link
Collaborator Author

shhyou commented Jan 4, 2025

Another attempt to just change make-visible in #708.

@shhyou shhyou linked a pull request Jan 4, 2025 that will close this issue
@rfindler
Copy link
Member

rfindler commented Jan 4, 2025

Thank you!

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 a pull request may close this issue.

2 participants