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

Add WASM-based Ormolu Live #964

Merged
merged 3 commits into from
Jan 24, 2023
Merged

Add WASM-based Ormolu Live #964

merged 3 commits into from
Jan 24, 2023

Conversation

amesgen
Copy link
Member

@amesgen amesgen commented Jan 6, 2023

New Ormolu Live based on the upcoming GHC WASM backend.

Closes #951

Test deployment at https://ormolu-live-testbed.netlify.app/

Diff is severly inflated by lockfile changes.

@amesgen
Copy link
Member Author

amesgen commented Jan 6, 2023

@asarkar As you have been giving feedback on the old Ormolu Live version, I would be interested in your thoughts, in particular if you think that the new version resolves #951.

https://ormolu-live-testbed.netlify.app/

@asarkar
Copy link

asarkar commented Jan 6, 2023

@amesgen Thanks for doing this, I’ll take a look later. One unrelated comment, can you display a verbiage on the page about the storage/usage of code that is being formatted? In other words, if you’re not saving any of the code or phoning home or using any part of it to train the tool, can you call it out, so that people formatting sensitive code know what to expect.

@amesgen amesgen force-pushed the amesgen/ormolu-live-wasm branch from 2244ce8 to 426a058 Compare January 6, 2023 18:46
@amesgen
Copy link
Member Author

amesgen commented Jan 6, 2023

One unrelated comment, can you display a verbiage on the page about the storage/usage of code that is being formatted? In other words, if you’re not saving any of the code or phoning home or using any part of it to train the tool, can you call it out, so that people formatting sensitive code know what to expect.

Good point, I could imagine that it might even be illegal under the GDPR to send the data without explicit consent. I added this line:

Note that this website is entirely client-side; in particular, your input is never sent to a remote server.

@asarkar
Copy link

asarkar commented Jan 7, 2023

I've had a chance to look at the new page, and here are my findings:

  1. The copy button on the right is gone, which, IMO, is a no-go.
  2. It's not clear which of the two panes is for the user. A note like "paste or type here" that disappears as soon as something is pasted or typed would make for a better user experience. For reference, see https://codebeautify.org/jsonvalidator and https://text-compare.com/.
  3. IMO, "See the GitHub repository" button is redundant because you're already linking to a commit in the GitHub repo.
  4. The disclaimer "this website is entirely client-side" is better suited as a footnote.
  5. Can you show the GHC version, perhaps as a second footnote?
  6. In addition to the rows, it'll be helpful to show the column at which the cursor is at. Here's why:

Input abc <- xs results in:

<input>:1:5-6
  The GHC parser (in Haddock mode) failed:
  parse error on input `<-'

Now, while the last line does say what the problem is, this is an exception rather than the rule as Haskell is notorious for hard-to-understand compiler errors. Column 5 happens to be the beginning of <-.

@amesgen amesgen force-pushed the amesgen/ormolu-live-wasm branch from 426a058 to 39b33c9 Compare January 8, 2023 17:54
@amesgen
Copy link
Member Author

amesgen commented Jan 8, 2023

I've had a chance to look at the new page, and here are my findings:

Thanks, I just updated the site based on your findings with only slight deviations (ghc-lib-parser version is next to the commit link, and I left off point 3 for now, as I find it somewhat neat to be able to directly jump to the README).

@asarkar
Copy link

asarkar commented Jan 8, 2023

LGTM 👍

@asarkar
Copy link

asarkar commented Jan 11, 2023

it'll be helpful to show the column

Just want to say that this was a great idea. I pasted the following code at https://ormolu-live.tweag.io/:

module Chapter03.Lib () where

class Monad m where
return :: a -> m a
(>>=) :: m a -> (a -> m b) -> m b

then_ :: Maybe a -> (a -> Maybe b) -> Maybe b

instance Monad Maybe where
  return = Just
  (>>=) = then_

-- Exercise3.1. Write the implementation of ap.
ap :: Monad m => m (b -> c) -> m b -> m c
ap f x = f >>= $ \f' -> x >>= $ \x' -> f' x'

and it failed with:

The GHC parser failed:

<input>:15:16

parse error on input `$'

However, column numbers aren't shown in the input, so, it's not immediately apparent which of the two $ is the problem. Thanks for implementing my suggestions.

@amesgen amesgen force-pushed the amesgen/flakeify branch 4 times, most recently from 23a13e2 to 3e7308c Compare January 20, 2023 18:27
@amesgen amesgen force-pushed the amesgen/ormolu-live-wasm branch from 39b33c9 to 1464810 Compare January 20, 2023 18:43
@github-actions
Copy link

github-actions bot commented Jan 20, 2023

@github-actions github-actions bot temporarily deployed to pull request January 20, 2023 19:15 Inactive
Base automatically changed from amesgen/flakeify to master January 23, 2023 17:10
Various re-exports got removed.
@amesgen amesgen force-pushed the amesgen/ormolu-live-wasm branch from 1464810 to 6d63645 Compare January 23, 2023 17:31
@github-actions github-actions bot temporarily deployed to pull request January 23, 2023 17:48 Inactive
Copy link
Member

@mrkkrp mrkkrp left a comment

Choose a reason for hiding this comment

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

Exciting stuff!

amesgen and others added 2 commits January 24, 2023 18:18
Should be reverted once the WASM backend supports TH.

Co-authored-by: Mark Karpov <mark.karpov@tweag.io>
Co-authored-by: Mark Karpov <mark.karpov@tweag.io>
@amesgen amesgen force-pushed the amesgen/ormolu-live-wasm branch from 1f80df9 to d824bc9 Compare January 24, 2023 17:19
@github-actions github-actions bot temporarily deployed to pull request January 24, 2023 17:34 Inactive
@mrkkrp mrkkrp merged commit 4d62a7e into master Jan 24, 2023
@mrkkrp mrkkrp deleted the amesgen/ormolu-live-wasm branch January 24, 2023 17:45
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.

Online formatter should allow copying parts of the output
3 participants