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

Render markdown in entity descriptions #1413

Merged
merged 14 commits into from Aug 12, 2023
Merged

Render markdown in entity descriptions #1413

merged 14 commits into from Aug 12, 2023

Conversation

xsebek
Copy link
Member

@xsebek xsebek commented Aug 6, 2023

  • use Markdown.Document as entityDescription
  • add missing spaces in chunksOf
  • fix code in entities.yaml (mostly types and few outdated snippets)
  • add code markdown in craft tutorial
  • use colours for types and entities

to compute a final result of type `b`{=type}. For example, if
`x`{=snippet}, `y`{=snippet}, and `z`{=snippet}
are nearby actors, then `meetAll f b0`{=snippet} is equivalent to
`b1 <- f b0 x; b2 <- f b1 y; f b2 z`{=snippet}.
Copy link
Member Author

Choose a reason for hiding this comment

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

It would be kind of neat if we could introduce variables with markup and then apply them to the rest of the document. Something like:

If `x`{=variable} is a nearby actor, then `give "apple" x` will end all hunger.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, that would be neat, and not too hard in theory, though it might require a bit of work to plumb the right information around. The basic idea would be that when we typecheck code in backticks we don't simply start it in an empty context, but rather start it in a context that already contains any declared variables (with types like forall a. a).

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

@byorgey indeed, I will make an Issue for it. 🙂

@kostmo I think we would need to:

  1. wrap pure expressions in return
  2. sequence everything in binds
  3. unsequence after check

Also some kind of mechanism to hide code, so we can declare variables without cluttering descriptions. I don't know Literate Haskell, so maybe there is a standard way to do so.

@xsebek
Copy link
Member Author

xsebek commented Aug 6, 2023

This is how I debugged it:

ghci> -- parse entities.yaml
ghci> em :: [Entity] <- either (fail . prettyPrintParseException) pure =<< decodeFileEither "data/entities.yaml"
ghci> -- format 3D printer descriptions like in TUI
ghci> let Just d3 = L.find (\e -> view entityName e == T.pack "3D printer") em in chunksOf 99 . toStream $ view entityDescription d3

And it finally seems to look OK:
Screenshot 2023-08-06 at 2 19 12

@@ -181,7 +216,8 @@ fromTextPure :: Text -> Either String (Document Text)
fromTextPure t = do
let spec = Mark.rawAttributeSpec <> Mark.defaultSyntaxSpec <> Mark.rawAttributeSpec
let runSimple = left show . runIdentity
runSimple $ Mark.commonmarkWith spec "markdown" t
Document tokenizedDoc <- runSimple $ Mark.commonmarkWith spec "markdown" t
return . Document $ normalise <$> tokenizedDoc
Copy link
Member

Choose a reason for hiding this comment

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

Random thought: would there be any value in having a NormalizedDocument newtype whose "smart constructor" performs the normalize function?

Copy link
Member Author

Choose a reason for hiding this comment

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

Honestly, if Document used Seq I would consider doing it immediately in (<>), the naive approach here merges the text one-by-one anyway.

@byorgey
Copy link
Member

byorgey commented Aug 6, 2023

And it finally seems to look OK:

The formatting looks great, but now it looks cramped. Can we add blank lines in between every paragraph, like we used to?

data/entities.yaml Outdated Show resolved Hide resolved
@byorgey
Copy link
Member

byorgey commented Aug 6, 2023

CI is failing since there is some code in a docspec test in Swarm.Game.Exception that needs to be fixed, I think.

@xsebek
Copy link
Member Author

xsebek commented Aug 11, 2023

The formatting looks great, but now it looks cramped. Can we add blank lines in between every paragraph, like we used to?

@byorgey should be fixed now, I had to rethink the code a little and keep paragraphs for printing.

@xsebek
Copy link
Member Author

xsebek commented Aug 11, 2023

Btw. the docspec test would not fail if we had OverloadedLists extension enabled, but as it is I used mempty.

But for markdown in Haskell code (e.g. generated entity/robot description) we can use that instance.

Copy link
Member

@kostmo kostmo left a comment

Choose a reason for hiding this comment

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

LGTM with minor suggestions. Also don't forget this one from @byorgey.

@@ -208,11 +251,8 @@ unStream = \case
TextNode a t -> (TextNode a, t)
CodeNode t -> (CodeNode, t)
RawNode a t -> (RawNode a, t)
ParagraphBreak -> error "Logic error: Paragraph break can not be unstreamed!"
Copy link
Member

Choose a reason for hiding this comment

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

Always nice to get rid of partial functions...

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, I don't think that paragraph break worked anyway! 😄 It shows that I wrote it before knowing where and how we use the documents.

src/Swarm/Language/Text/Markdown.hs Show resolved Hide resolved
src/Swarm/Language/Text/Markdown.hs Outdated Show resolved Hide resolved
@xsebek
Copy link
Member Author

xsebek commented Aug 12, 2023

@kostmo I went ahead and added some colours anyway:
Screenshot 2023-08-12 at 12 04 07

I also added a script to run scenarios in order with --autoplay and show goal descriptions. 😁

❯ scripts/autoplay-tutorials.sh
backstory.yaml	CONTINUE [Y/n]:
move.yaml	CONTINUE [Y/n]:
craft.yaml	CONTINUE [Y/n]:
grab.yaml	CONTINUE [Y/n]:
place.yaml	CONTINUE [Y/n]: n

@xsebek xsebek added the merge me Trigger the merge process of the Pull request. label Aug 12, 2023
@mergify mergify bot merged commit f743c90 into main Aug 12, 2023
10 checks passed
@mergify mergify bot deleted the markdown-entities branch August 12, 2023 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merge me Trigger the merge process of the Pull request.
Projects
None yet
3 participants