greghendershott / racket-mode

Emacs major and minor modes for Racket: edit, REPL, check-syntax, debug, profile, packages, and more.
https://www.racket-mode.com/
GNU General Public License v3.0
682 stars 93 forks source link

printing in rhombus is incorrect #725

Open samth opened 3 days ago

samth commented 3 days ago

Please copy all of the following lines and paste them into your bug report at https://github.com/greghendershott/racket-mode/issues/.

Package

metadata
nil
package-archives
(("elpa" . "https://elpa.gnu.org/packages/")
 ("melpa" . "https://melpa.org/packages/"))
racket--el-source-dir
"/home/samth/sw/racket-mode/"
racket--rkt-source-dir
"/home/samth/sw/racket-mode/racket/"

System values

emacs-version
"29.3"
major-mode
racket-hash-lang-mode
system-type
gnu/linux
x-gtk-use-system-tooltips
t
display-graphic-p
t

Buffer values

after-change-functions
(jit-lock-after-change t racket--xp-after-change-hook racket--hash-lang-after-change-hook)
before-change-functions
(t syntax-ppss-flush-cache)
completion-at-point-functions
(racket-xp-complete-at-point)
eldoc-documentation-function
eldoc-documentation-default
eldoc-documentation-strategy
eldoc-documentation-default
eldoc-documentation-functions
(racket-xp-eldoc-point racket-xp-eldoc-sexp-app t)
font-lock-defaults
(nil t)
pre-command-hook
(eldoc-pre-command-refresh-echo-area t)
post-command-hook
(eldoc-schedule-timer t)
post-self-insert-hook
(racket-hash-lang-post-self-insert t)
xref-backend-functions
(racket-xp-xref-backend-function t)

Racket Mode values

racket--cmd-open-p
t
racket-after-run-hook
nil
racket-back-end-configurations
((:directory "/" :racket-program nil :remote-source-dir nil :restart-watch-directories nil :windows nil))
racket-before-run-hook
(racket-ansi-color-context-reset)
racket-browse-url-function
browse-url
racket-command-timeout
10
racket-documentation-search-location
"https://docs.racket-lang.org/search/index.html?q=%s"
racket-error-context
medium
racket-expand-hiding
standard
racket-hash-lang-mode-hook
(racket-xp-mode)
racket-hash-lang-module-language-hook
nil
racket-hash-lang-token-face-alist
((constant . font-lock-constant-face)
 (error . error)
 (other . font-lock-doc-face)
 (keyword . font-lock-keyword-face)
 (hash-colon-keyword . racket-keyword-argument-face)
 (at . font-lock-doc-face))
racket-history-filter-regexp
"\\`\\s *\\'"
racket-imagemagick-props
nil
racket-images-do-not-use-svg
nil
racket-images-inline
t
racket-images-keep-last
100
racket-images-system-viewer
"display"
racket-indent-curly-as-sequence
t
racket-indent-sequence-depth
0
racket-logger-config
((racket-hash-lang-mode . debug)
 (racket-mode . debug)
 (cm-accomplice . warning)
 (GC . info)
 (module-prefetch . warning)
 (optimizer . info)
 (racket/contract . error)
 (racket-mode-debugger . info)
 (sequence-specialization . info)
 (* . fatal))
racket-memory-limit
2048
racket-mode-hook
(racket-xp-mode)
racket-module-forms
"\\s(\\(?:module[*+]?\\|library\\)"
racket-pretty-lambda
nil
racket-pretty-print
t
racket-program
"~/bin/racket"
racket-repl-buffer-name-function
nil
racket-repl-command-file
"/home/samth/.emacs.d/racket-mode/repl.rkt"
racket-repl-history-directory
"~/.emacs.d/racket-mode/"
racket-repl-mode-hook
nil
racket-sexp-comment-fade
0.5
racket-shell-or-terminal-function
racket-shell
racket-show-functions
(racket-show-pseudo-tooltip)
racket-smart-open-bracket-enable
nil
racket-submodules-to-run
((test)
 (main))
racket-use-repl-submit-predicate
nil
racket-xp-add-binding-faces
t
racket-xp-after-change-refresh-delay
1
racket-xp-highlight-unused-regexp
"^[^_]"
racket-xp-mode-lighter
(:eval
 (racket--xp-mode-lighter))

Minor modes

enabled
((auto-composition-mode)
 (auto-compression-mode)
 (auto-encryption-mode)
 (blink-cursor-mode)
 (eldoc-mode)
 (file-name-shadow-mode)
 (font-lock-mode)
 (global-eldoc-mode)
 (global-font-lock-mode)
 (global-paren-face-mode)
 (indent-tabs-mode)
 (line-number-mode)
 (menu-bar-mode)
 (mouse-wheel-mode)
 (override-global-mode)
 (racket-xp-mode)
 (semantic-minor-modes-format)
 (shell-dirtrack-mode)
 (show-paren-mode)
 (tooltip-mode)
 (transient-mark-mode))
Disabled minor modes
disabled
((abbrev-mode)
 (auto-fill-function)
 (auto-fill-mode)
 (auto-save-mode)
 (auto-save-visited-mode)
 (buffer-face-mode)
 (buffer-read-only)
 (button-mode)
 (cl-old-struct-compat-mode)
 (column-number-mode)
 (comint-fontify-input-mode)
 (compilation-minor-mode)
 (compilation-shell-minor-mode)
 (completion-in-region-mode)
 (context-menu-mode)
 (cursor-face-highlight-mode)
 (defining-kbd-macro)
 (diff-auto-refine-mode)
 (diff-minor-mode)
 (electric-indent-mode)
 (electric-layout-mode)
 (electric-pair-mode)
 (electric-quote-mode)
 (global-prettify-symbols-mode)
 (global-semantic-highlight-edits-mode)
 (global-semantic-highlight-func-mode)
 (global-semantic-show-parser-state-mode)
 (global-semantic-show-unmatched-syntax-mode)
 (global-semantic-stickyfunc-mode)
 (global-visual-line-mode)
 (horizontal-scroll-bar-mode)
 (hs-minor-mode)
 (isearch-fold-quotes-mode)
 (isearch-mode)
 (jit-lock-debug-mode)
 (lock-file-mode)
 (lost-selection-mode)
 (next-error-follow-minor-mode)
 (overwrite-mode)
 (paragraph-indent-minor-mode)
 (paren-face-mode)
 (prettify-symbols-mode)
 (racket-hash-lang-repl-mode)
 (racket-smart-open-bracket-mode)
 (read-extended-command-mode)
 (semantic-highlight-edits-mode)
 (semantic-highlight-func-mode)
 (semantic-mode)
 (semantic-show-parser-state-mode)
 (semantic-show-unmatched-syntax-mode)
 (semantic-stickyfunc-mode)
 (sh-electric-here-document-mode)
 (shell-highlight-undef-mode)
 (size-indication-mode)
 (tab-bar-history-mode)
 (tab-bar-mode)
 (temp-buffer-resize-mode)
 (text-scale-mode)
 (tool-bar-mode)
 (treesit-explore-mode)
 (treesit-inspect-mode)
 (undelete-frame-mode)
 (url-handler-mode)
 (use-hard-newlines)
 (vc-dir-git-mode)
 (view-mode)
 (visible-mode)
 (visual-line-mode)
 (window-divider-mode)
 (xref-etags-mode))

Steps to reproduce:

Write a rhombus file like this:

#lang rhombus
class M(x)
M(1)

Run it in racket-mode.

It prints like this:

(M 1)

Run the file with racket at the command line.

It prints like this:

M(1)

My theory is that racket-mode is not calling configure-runtime correctly but I have not investigated this at all.

greghendershott commented 3 days ago

Thanks for the report.

Although I'm 99% sure configure-runtime is being called, I'll double check.

The bug seems to require that the Emacs customization variable racket-pretty-print is the default t, which causes the back end to use pretty-print. I can reproduce with that, but not with plain print, which IIUC is what command-line racket would do.

Although that narrows it down a little, I'm not yet sure exactly where/what the problem is, and who needs to solve it, how. But I'll look...

greghendershott commented 2 days ago

What I've been able to figure out so far:

rhombus' configure-runtime sets a special value for global-port-print-handler procedure. This is happening in Racket Mode. (Amusingly I can see it affecting log-debug ~v values, from Racket code.)

However that's N/A -- the current-print parameter is what is used by (a) REPL printing and (b) #%module-begin. The latter is applicable when you run your example program.


In addition to struct props for individual values, there is a small zoo of print-ish handlers:

I am not 100% confident which are intended to be used by (a) end user programs, (b) languages, and (c) tools.

I do know that Racket mode (re)sets these before each run, but doesn't touch values changed by the language, libs, and/or end user program (which AFAICT is the right thing to do?).


I am not sure but maybe rhombus ought to set current-print and perhaps pretty-print-{print size}-hook functions? Or at least change its #%module-begin not to use current-print?? Again I'm not 100% clear what's intended.

samth commented 2 days ago

Just calling Racket's pretty-print in Rhombus results in the same printing as I see in racket-mode:

#lang rhombus

import lib("racket/main.rkt") as r:
  expose #{pretty-print} as pretty_print

class M(x)
pretty_print(M(1))

I'm not sure what exactly the right configuration should be here, maybe @mflatt would have a suggestion. Alternatively, maybe just drop automatic pretty-print for rhombus files?

greghendershott commented 2 days ago

Alternatively, maybe just drop automatic pretty-print for rhombus files?

Yes. In fact it occurs to me now that racket/pretty is thoroughly s-expression oriented. I recall seeing code in rhombus suggesting it has its own concept of pretty printing, which makes sense.

I'd love that to be lang-driven not ad hoc, if possible. That is, some way for a lang to say, "If a user prefers pretty-printing for module-begin and REPL, my global print handler already does that by default, just use that." For example.

p.s. I think it's also possible that racket/pretty might have a bug where it's not using the global print handler, somehow? But that might be moot, here.

greghendershott commented 2 days ago

Thinking about this a little bit more, maybe I should use pretty-print only if the user prefers that, and default-global-port-print-handler is in effect:

 (define (make-racket-mode-print-handler pretty? columns pixels/char)
   (define (racket-mode-print-handler v)
     (unless (void? v)
       (define-values (in out) (make-value-pipe))
       (parameterize ([current-output-port out]
                      [print-syntax-width +inf.0])
         (cond
-          [pretty?
+          [(and pretty?
+                (equal? (global-port-print-handler)
+                        default-global-port-print-handler))
            (parameterize ([pretty-print-columns columns]
                           [pretty-print-size-hook (make-pp-size-hook pixels/char)]
                           [pretty-print-print-hook (make-pp-print-hook)])
              (pretty-print v))]
           [else
            (match (convert-image v)
              [(cons path-name _pixel-width)
               (write-special (cons 'image path-name))]
              [_
               (print v)])
            (newline)]))
       (drain-value-pipe in out)))
   racket-mode-print-handler)

In a quick test, this solves the immediate issue.

The rationale seems... plausible? "If a language has changed the default global port print handler, we can't assume it's an s-expression language and pretty-print is appropriate. In fact it's best not to assume anything at all -- just use print and let the handler do its thing."

Anyway that's my least-worst idea, so far. I'll ponder it a bit more. Of course I'm also open to better ideas or advice.

greghendershott commented 2 days ago

Although I might hold off in case anyone has better ideas, I might go ahead and merge commit bceac44.

It does help with the reported problem.

Admittedly pretty printing would be disabled if something changed the global port print handler, for any reason -- even if the values could still be pretty-printed as s-expressions. So that's a downside. But the values would still be printed.

[I could also enhance the racket-pretty-print variable from a boolean, to something more complicated, such as three values (always, never, only if default global print handler), or even a dictionary from module language names to such settings. But... I'd prefer to do that with a real-world use-case and user to confirm it works for them, as opposed to proactively.]

samth commented 2 days ago

Just as an additional note, the racket repl uses pretty-print and has this problem as well.

greghendershott commented 1 day ago

Just as an additional note, the racket repl uses pretty-print and has this problem as well.

Interesting. It looks like:

  1. The default value of current-print where it's defined in racket/src/io/run/main.rkt is just print + newline. And if you $ racket -l racket/base -i, current-print in the REPL is that plain, non-pretty print.

  2. However racket/collects/racket/init.rkt sets current-print to a pretty-printer procedure that calls pretty-print. Which is what you get from the REPL for $ racket, which opens in the full racket lang.

greghendershott commented 1 day ago

I suppose one could say, "Well, you should use $ racket -l rhombus -i".

On the other hand people do things like run $ racket, then use xrepl's ,enter command one or more times during a session, which could be a program using any module language. I think the expectation is this just "just work". (And that's closer to the model for things like Racket Mode, DrRacket, and other such tools.)


So it seems current-print needs to be lang-driven, somehow. Or pretty-print needs to be lang-driven. Or there's some new current-pretty-printer parameter, which can be racket/pretty's pretty-print, or whatever else the lang decides. Or... something...

greghendershott commented 1 day ago

On the other hand people do things like run $ racket, then use xrepl's ,enter command one or more times during a session, which could be a program using any module language.

Well. You might use ,enter to run a #lang rhombus program... once. Thereafter, you're stuck. AFAICT. The xrepl command trick ("we can use , because a stand-alone quasiunquote isn't legal") doesn't work anymore, understandably for rhombus syntax.

p.s. Also I couldn't ,exit. (Not even C-c worked. I had to kill the terminal shell window.)

mflatt commented 6 hours ago

No real answers, but some possibly relevant information.

The Racket REPL uses pretty-print because the default -I is -I racket/init, and it's racket/init that installs the pretty printer. To work in Rhombus mode, I use -I rhombus, which does not install the racket/pretty printer, but does install a Rhombus pretty printer via configure-runtime. Note that -I racket would not set the printer, because its configure-runtime doesn't do that.

DrRacket injects pretty printing by setting global-port-print-handler, as opposed to current-print. The rhombus configure-runtime submodule replaces that global port print handler with one that uses Rhombus printing. Meanwhile, note that racket/init sets current-print, but rhombus (via configure-runtime) sets global-port-print-handler.

XREPL recognizes ,exit and similar even when starting with -I rhombus. That's because XREPL checks an input string for whitespace followed by ,. The shrubbery syntax and reader cooperate slightly by agreeing that something like ,exit is ready to evaluate (as opposed to ,(, for example), even though it would read as an error.

Using ,enter in XREPL uses enter!, which does not attempt to load configure-runtime or similar, and the configure-runtime protocol is not really set up for going into different modules. The configure-expand protocol is more set up for that with enter-parameterization and exit-parameterization, which is more compatible with the idea of switching contexts. But the protocol does that by swapping whole parameterizations; I'm not sure that's the right idea for run-time swapping, but maybe it would work. In short, to make a REPL adapt to different contexts, probably we'd want.a new submodule with a enter—exit protocol, and maybe the configure-expand approach would be good for that.