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: use port-name-matches? for symbols #708

Merged
merged 1 commit into from
Jan 5, 2025

Conversation

shhyou
Copy link
Collaborator

@shhyou shhyou commented Jan 4, 2025

When given symbols, they could be names for unsaved buffers. Directly match the definition window and the interaction window instead. Otherwise, fallback to find-matching-tab for path-strings? like before.

Related to d27a480.

@shhyou shhyou linked an issue Jan 4, 2025 that may be closed by this pull request
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resyntax analyzed 1 file in this pull request and found no issues.

@rfindler
Copy link
Member

rfindler commented Jan 4, 2025

I'm not sure if I'm missing something here, but I think that port-name-matches? is supposed to accept the path string and work with that (and that'll work when the file has the saved name)?

@shhyou
Copy link
Collaborator Author

shhyou commented Jan 4, 2025

I don't know. The new code is simply to guarantee that the existing code path does not change because I don't understand the code.

@rfindler
Copy link
Member

rfindler commented Jan 4, 2025

I did only some basic testing, but maybe something like this?

$  git diff | cat
diff --git a/drracket/drracket/private/unit.rkt b/drracket/drracket/private/unit.rkt
index e25fa757..334103af 100644
--- a/drracket/drracket/private/unit.rkt
+++ b/drracket/drracket/private/unit.rkt
@@ -3419,33 +3419,29 @@
                     (path->string (normal-case-path (normalize-path p2))))))
 
       (define/override (make-visible filename #:start-pos [start-pos #f] #:end-pos [end-pos start-pos])
-        (let ([tab (find-matching-tab filename)])
-          (when tab
-            (change-to-tab tab)
-            (when (and start-pos end-pos)
-              (define (set-the-position ed)
-                (when (send ed port-name-matches? filename)
-                  (send (send ed get-canvas) focus)
-                  (send ed set-caret-owner #f)
-                  (send ed set-position start-pos end-pos)))
-              (set-the-position (send tab get-defs))
-              (set-the-position (send tab get-ints))))))
+        (match (find-matching-tab/which-editor filename)
+          [(cons tab ed)
+           (change-to-tab tab)
+           (when (and start-pos end-pos)
+             (send (send ed get-canvas) focus)
+             (send ed set-caret-owner #f)
+             (send ed set-position start-pos end-pos))]
+          [#f (void)]))
       
       (define/public (find-matching-tab filename)
-        (define fn-path (if (string? filename)
-                            (string->path filename)
-                            filename))
+        (match-define (cons tab ed) (find-matching-tab/which-editor filename))
+        tab)
+
+      (define/private (find-matching-tab/which-editor 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)))
+          (define (try ed)
+            (and (send ed port-name-matches? filename)
+                 (cons tab ed)))
+          (or (try (send tab get-defs))
+              (try (send tab get-ints)))))
       
       (define/override (editing-this-file? filename)
-        (ormap (λ (tab)
-                 (or (send (send tab get-defs) port-name-matches? filename)
-                     (send (send tab get-ints) port-name-matches? filename)))
-               tabs))
+        (and (find-matching-tab/which-editor filename) #t))
 
       (define/override (get-all-open-files)
         (filter

I can push that to your branch, I think, if you want to try it? What do you think?

@shhyou
Copy link
Collaborator Author

shhyou commented Jan 5, 2025

Sure! Please feel free to apply any changes. I don't have any idea about the appropriate code organization.

@shhyou
Copy link
Collaborator Author

shhyou commented Jan 5, 2025

Does port-name-matches? convert strings to paths before matching the given name?

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resyntax analyzed 1 file in this pull request and found no issues.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resyntax analyzed 1 file in this pull request and found no issues.

@shhyou
Copy link
Collaborator Author

shhyou commented Jan 5, 2025

This is ready, I think?

@rfindler
Copy link
Member

rfindler commented Jan 5, 2025

Does port-name-matches? convert strings to paths before matching the given name?

It doesn't! And I see there was some conversion code in there before my change. I'll bring that back and merge it.

@rfindler rfindler force-pushed the make-visible-port-name-matches branch from a878209 to b825a81 Compare January 5, 2025 21:32
And fallback to find-matching-tab for path-strings? like before.

Releated to racket/drracket@d27a480.
@rfindler rfindler force-pushed the make-visible-port-name-matches branch from b825a81 to 23130b5 Compare January 5, 2025 21:35
@rfindler rfindler merged commit 470c745 into racket:master Jan 5, 2025
3 checks passed
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resyntax analyzed 1 file in this pull request and found no issues.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Resyntax analyzed 1 file in this pull request and found no issues.

@shhyou shhyou deleted the make-visible-port-name-matches branch January 6, 2025 02:33
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.

make-visible not finding unsaved-editor
2 participants