Skip to content

Conversation

@mikesperber
Copy link
Member

To that end, call the underlying pretty-print-print-handler instead of format, and transform write-special'ed number markup into the corresponding snip.

@mikesperber mikesperber requested a review from jbclements April 29, 2025 10:58
@mikesperber
Copy link
Member Author

I should not that the snips don't look very good, but this needs to be fixed in the framework.

@rfindler
Copy link
Member

rfindler commented Apr 29, 2025 via email

@mikesperber
Copy link
Member Author

Is there a place to add a test case for this one? Robby

A meaningful test for this would have to look at the actual live UI of the stepper - I haven't seen that yet. (Or am I wrong, @jbclements ?)

@jbclements
Copy link
Contributor

@mike no, there's no way to look at the live GUI output that I know of, though I can add a manual checklist item to manual-tests. You probably have an easy example that I can test?

Broadly speaking: this definitely seems like an improvement.

Copy link
Contributor

@jbclements jbclements left a comment

Choose a reason for hiding this comment

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

It certainly seems like the existing code ignored the existing pretty-print-size-hook, and I can't see why that would be a good thing. The direct use of format with ~s definitely seems less likely to be correct.

@mikesperber
Copy link
Member Author

@rfindler Any chance you could help with making the number snips use the same font size as the stepper-sub-text%. I've tried doing set-style on the same style as in the stepper-text%'s snip, to no avail.

@rfindler
Copy link
Member

rfindler commented May 8, 2025

In addition to the font size, the number snips don't follow the settings in the language dialog for the language-dialog version of BSL. When I change the setting to "Mixed fractions" in the language dialog customization of BSL, I still see repeating-decimals printing in the stepper. (It might depend on my preferences, tho, so you might need to use "Repeating decimals" to get the bad behavior.)

@rfindler
Copy link
Member

rfindler commented May 8, 2025

Also, when I am trying things out, sometimes I see "placeholder26196" in the stepper where the fraction is supposed to go. I'm not sure how to make this happen but maybe it has to do with whether or not the language-based BSL is chosen when DrRacket starts up?

@mikesperber
Copy link
Member Author

@rfindler For me, the stepper respects the preferences. (And the code for that is unchanged.)
With the placeholder thing you're seeing - the patch only fiddles with number markup, AFAICS. Do you have a reproducible recipe?

Also, do you have any advice on how to fix font size?

@mikesperber
Copy link
Member Author

Logging here that I figured out what @rfindler was seeing: For the menu-based languages, the stepper doesn't receive the language-specific pretty-print hooks, and then everything goes to hell. So that also needs to be fixed.

To that end, call the underlying pretty-print-print-handler
instead of format, and transform write-special'ed number markup
into the corresponding snip.
@mikesperber mikesperber force-pushed the respect-number-format branch from 2c1e16a to a56ccc9 Compare June 6, 2025 13:58
@rfindler
Copy link
Member

rfindler commented Jun 6, 2025

After format-sexp completes, is there something that makes sure the data actually got flushed out of the port to the editor? I'm not sure what would guarantee that (or if it is necessary actually), but maybe closing the port? (I don't see anything in the docs about it.)

@rfindler
Copy link
Member

It seems to me that the way the stepper plugs into drracket when it is being used in the menu-based teaching languages needs to make sure that the pretty-print hooks make their way over so they are set up by the time we get to format-sexp, just as Mike says.

It looks like information about the specific language are coming into stepper-sub-text% via init fields, like show-inexactness? so maybe the place that is supplying these needs to also supply other parameter values?

@rfindler
Copy link
Member

I see that there is the function configure/settings that sets up everything. Does it make sense to try to use that function to initialize the environment for use in the stepper, @jbclements ?

@mikesperber
Copy link
Member Author

I see that there is the function configure/settings that sets up everything. Does it make sense to try to use that function to initialize the environment for use in the stepper, @jbclements ?

@rfindler Yes, but for calling it the code requires access to the Output Syntax settings approximately here:

(if (implementation? % stepper-language<%>)

Note that calling drscheme:rep:current-language-settings doesn't work - presumably because we're not in a REPL context.

@rfindler
Copy link
Member

IIUC, the stepper has several object/classes for the teaching languages (one for each). I'm suggesting that that code in those objects call the configure/settings function.

@mikesperber
Copy link
Member Author

IIUC, the stepper has several object/classes for the teaching languages (one for each). I'm suggesting that that code in those objects call the configure/settings function.

@rfindler I understand that - but to call it, I need the settings. (The settings is not the language, but - in this case - the output format.)

@rfindler
Copy link
Member

Looking a little bit more here, maybe the right thing is for the stepper to call the configure function. It's argument looks to be an association list that can be built from the information that each of the language-specific objects that the stepper creates.

I'm sorry -- I thought I'd found a place in the stepper where those objects are created to try to track down if the information can be plumbed though from those to this point but I'm not seeing it anymore. @jbclements does what we are saying here make any sense?

@mikesperber
Copy link
Member Author

Looking a little bit more here, maybe the right thing is for the stepper to call the configure function. It's argument looks to be an association list that can be built from the information that each of the language-specific objects that the stepper creates.

I'm afraid I have to repeat myself - the two configure functions are basically equivalent, except for the format of their inputs - so that function also needs the settings.

@mikesperber
Copy link
Member Author

@rfindler The code path you probably want to know starts here:

(send tab stepper-button-callback

... and then goes here:
(define/public (stepper-button-callback language settings)

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.

3 participants