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

Restrict the type of the last expression in at-eval blocks #1919

Merged
merged 5 commits into from
Sep 11, 2022

Conversation

mortenpi
Copy link
Member

@mortenpi mortenpi commented Sep 5, 2022

Currently, you are generally allowed to have at-eval to return anything, and the resulting object will get spliced into the document somehow, but we do not really define how we treat arbitrary objects (HTML just reprs them and dumps the resulting string as text in the document).

In practice, and based on our docs, most of the time you're expected to either return nothing (so nothing gets displayed) or Markdown.MD (so you get properly rendered Markdown). This makes that requirement general and explicit, making it easier to support this block in the writers (that now know that it will always be either nothing or Markdown.MD, and don't have to deal with arbitrary types).

This can be breaking because it was possible to get somewhat reasonable output for some return types and it's possible that people are using it (in particular, I am concerned about cases where people might be returning various Markdown.* nodes without wrapping them in Markdown.MD). For that reason, this does not make that case quite an error, but instead you get a warning, and the resulting object is showed into a text representation and displayed as a code block (so there is behavior change for users).

@fredrikekre
Copy link
Member

Perhaps possible to display the same as an example block?

@mortenpi
Copy link
Member Author

mortenpi commented Sep 8, 2022

Yeah, I agree, I think eventually we want to treat this the same as at-example. But I would stick to this change for now, to make my life a little easier in #1912 and in its HTML counterpart. Properly rendering non-Nothing/Markdown.MD elements can be done as a non-breaking enhancement in the future.

@fredrikekre
Copy link
Member

Yea, and maybe it makes sense to keep that restriction. If you want an example block you can just use an example block (the only advantage would be hiding the code by default I suppose).

@mortenpi mortenpi merged commit 845bf98 into master Sep 11, 2022
@mortenpi mortenpi deleted the mp/restrict-eval branch September 11, 2022 05:04
@mortenpi
Copy link
Member Author

Yea, and maybe it makes sense to keep that restriction. If you want an example block you can just use an example block (the only advantage would be hiding the code by default I suppose).

Let's leave that as an open issue for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants