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

Purescript style guide #56

Open
ngua opened this issue Jan 27, 2022 · 10 comments
Open

Purescript style guide #56

ngua opened this issue Jan 27, 2022 · 10 comments
Labels
lower-priority Items that do not belong to a particular stage and are not critical

Comments

@ngua
Copy link
Contributor

ngua commented Jan 27, 2022

We have a Haskell style guide which we can partially follow, but we should probably have one for Purescript code as well (and maybe even for FFI code)

Let's use this issue as a list of conventions we try to follow

  • Avoid newtypes when there are no type class instances for the type, except of cases where we want to use newtypes to prevent confusion when working with multiple values of the same type.
  • No explicit instance names
  • No $ required before do, unlike in Haskell
  • Don't mix $ with parentheses where it's possible to use only $s
  • Prefer f1 $ f2 $ f3 v over f1 <<< f2 <<< f3 $ v and (f1 <<< f2 <<< f3) v (compose uses more stack and has longer operator name)
  • Naming of FFI functions should start with underscore, except of cases when the function is exported right away with no changes to the public interface
  • If the goal of an import is just to re-export, use import Foo.Bar (baz) as X, not import Foo.Bar (baz) as Foo.Bar (avoids boilerplate and clearly indicates the purpose of the import)
  • Do not export functions more than once from Contract.* modules. The motivation is it would make imports management more complicated for the users (the need to choose between modules). language-server users will have a better UX (there is no need to choose a module if only one module exports an identifier)
  • Do not import modules from test/ in src/ (the reason is: spago only looks up for modules from src/ in dependencies of a project, so that would make CTL unusable as dependency).
@klntsky
Copy link
Member

klntsky commented Jan 28, 2022

This one looks good for me, though it is a bit dated (effect types are not used anymore). Also I don't see a valid rationale for Doubly indent nested record or array literals .

@ngua
Copy link
Contributor Author

ngua commented Jan 29, 2022

Hmmm, I don't understand that one either. I think some of the guidelines won't work with purty as well. It's probably a good starting point though, thanks for sharing it

@ngua ngua added the lower-priority Items that do not belong to a particular stage and are not critical label Jan 31, 2022
@klntsky
Copy link
Member

klntsky commented Jan 31, 2022

Since we have an autoformatter now, can we just use it as an oracle instead of writing a guide?

@vvtran vvtran mentioned this issue Feb 2, 2022
@adamczykm
Copy link
Contributor

I think there's more to style guide than formatting. We should probably have one that is based on Haskell one where possible.

@klntsky
Copy link
Member

klntsky commented Feb 4, 2022

Some PS-specific questions to address:

  • should we write instance names: NO
  • naming of FFI functions

@ngua ngua moved this to Todo in cardano-transaction-lib Jun 3, 2022
@klntsky klntsky pinned this issue Oct 5, 2022
@uhbif19
Copy link
Contributor

uhbif19 commented Nov 30, 2022

@klntsky Should not it be closed, as the matter is settled?

@klntsky
Copy link
Member

klntsky commented Nov 30, 2022

@uhbif19 the list in the original post is about things not eforced by the formatter

@rynoV
Copy link
Contributor

rynoV commented Dec 14, 2022

How about:

  • Prefer f1 $ f2 $ f3 v over f1 <<< f2 <<< f3 $ v and (f1 <<< f2 <<< f3) v

I think the first is preferable for purescript since the compose function is more characters.

@klntsky
Copy link
Member

klntsky commented Dec 15, 2022

@rynoV I agree, also, compose uses more stack.

@rynoV
Copy link
Contributor

rynoV commented Dec 24, 2022

Suggestion:

  • Prefer not $ f a b over not f a b. The second will generate more complex JS code and use more stack.

I haven't checked this, but based on the definition that would be used for the latter, I think recursively resolving the instances would be less efficient.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lower-priority Items that do not belong to a particular stage and are not critical
Projects
Development

No branches or pull requests

5 participants