unisonweb / elm-browser

A Unison Github repo explorer in Elm
MIT License
34 stars 4 forks source link

Consider not using elm-ui for code formatting. #34

Open zenhack opened 4 years ago

zenhack commented 4 years ago

The current elm-ui pretty printer does some weird things with layout, and it's a little hard to write pretty printers with it. It might be easier to just put strings together when formatting the code and just wrap the result in a <pre> tag. Talked to @ashishsc about this a bit. Thoughts?

ashishsc commented 4 years ago

For just textual layout for sure, but if we start making the terms clickable and more fancy then I think elm-ui layout is fine

mitchellwrosen commented 4 years ago

Totally agree! I was only using elm-ui to print terms because it was convenient. But it produces wonkly looking things, and should just be replaced, probably sooner rather than later.

mitchellwrosen commented 4 years ago

(Assuming whatever we use instead will allow us to do fancy on-hover things etc)

ashishsc commented 4 years ago

The hover element can be a sibling to the text blob. But if we want hover on each term in the code blob,then they need to be wrapped in an el

On Sat, Sep 7, 2019, 9:14 PM Mitchell Rosen notifications@github.com wrote:

(Assuming whatever we use instead will allow us to do fancy on-hover things etc)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/unisonweb/elm-browser/issues/34?email_source=notifications&email_token=AAUBCCXQQ3RXM753U2AF4ELQIRGXBA5CNFSM4IURKZIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6FFMYQ#issuecomment-529159778, or mute the thread https://github.com/notifications/unsubscribe-auth/AAUBCCTRO3M4ZDINW5YUEJDQIRGXBANCNFSM4IURKZIA .

zenhack commented 4 years ago

Using a <pre> tag also doesn't necessarily bar us from using other markup inside; we can still have links/do highlighting and stuff.

Quoting Ashish Chandwani (2019-09-08 09:53:29)

The hover element can be a sibling to the text blob. But if we want hover on each term in the code blob,then they need to be wrapped in an el On Sat, Sep 7, 2019, 9:14 PM Mitchell Rosen notifications@github.com wrote:

(Assuming whatever we use instead will allow us to do fancy on-hover things etc)

� You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub

https://github.com/unisonweb/elm-browser/issues/34?email_source=notifi cations&email_token=AAUBCCXQQ3RXM753U2AF4ELQIRGXBA5CNFSM4IURKZIKYY3PNVW WK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6FFMYQ#issuecomme nt-529159778, or mute the thread

https://github.com/notifications/unsubscribe-auth/AAUBCCTRO3M4ZDINW5YU EJDQIRGXBANCNFSM4IURKZIA .

-- You are receiving this because you authored the thread. Reply to this email directly, [1]view it on GitHub, or [2]mute the thread.

Verweise

  1. https://github.com/unisonweb/elm-browser/issues/34?email_source=notifications&email_token=AAGXYPX545M3IEPHNDAU5YLQIT7VTA5CNFSM4IURKZIKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6FQN2A#issuecomment-529204968
  2. https://github.com/notifications/unsubscribe-auth/AAGXYPXWF7Q2SH6LZSHF53TQIT7VTANCNFSM4IURKZIA
ashishsc commented 4 years ago

Ah yeah that's true.

zenhack commented 4 years ago

Heads up: I started working on this on a brance (pre-pp on my fork). Still very WIP.

zenhack commented 3 years ago

Unassigned myself, as I'm not likely to work on this at this point.