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

Move some of the printer logic to Easy Format #987

Closed
3 tasks
yunxing opened this issue Jan 19, 2017 · 4 comments
Closed
3 tasks

Move some of the printer logic to Easy Format #987

yunxing opened this issue Jan 19, 2017 · 4 comments

Comments

@yunxing
Copy link
Contributor

yunxing commented Jan 19, 2017

The current API in Easy Format has it's limitations, which require us to do a lot of hacks in our printer to support special use cases. It would be great if we can move those features into Easy Format's API, that would make our life much easier.

The wishlist includes:

  • Being able to add items after separators:
    This is mainly for end of line comments. Consider a list with "," as separator:
     {
       x, /* blah */
       y, 
      }

To format this using, current Easy format API looks like :

Makelist ~sep:"," ~children:["x", "y"]

There is no clean way in the current API to add comment after x. Instead, we have to transform the API call into something like:

makelist ~sep:"", ~children:[ makelist ~children:["x", ",", "/* blah */")]], makelist ~children:["y", ","]]

, which is ugly and hard to maintain.

  • Make trailing separators optional:
    In Reason we want
{
  x, 
  y
}

Currently we can't use the clean api, instead we need to apply a hack like:

makelist ~sep:"", ~children:[ makelist ~children:["x", ",", "/* blah */")]], "y"]

I wish we can have a api to say:

Makelist ~sep:"," ~lastSep:false ~children:["x", "y"]

The challenge is to nicely design the APIs listed above.

@chenglou
Copy link
Member

cc @bobzhang

@IwanKaramazow
Copy link
Contributor

Since it's one file, should we include it in core Reason's src?

@yunxing
Copy link
Contributor Author

yunxing commented Jan 20, 2017

I'm ok with forking and vendoring, especially when we probably won't have much updates from Easy Format in the near future.

@IwanKaramazow
Copy link
Contributor

Some thoughts from #41. I think we might need to extend EasyFormat to attach newlines above nodes (lists, labels?). I'm currently manually inserting (atom ""), which doesn't feel like the best way in the world.

Usecase:

module Page = {
  include ReactRe.Component;

  type props = {message: string};
  type state = {count: int};

  let format = "a4";
  let name = "Page";

  let handleClick {props} event => {
    Js.log "clicked!";
    None
  };

  let render {props, updater} =>
    <div onClick=(updater handleClick)> (ReactRe.stringToElement props.message) </div>;
};

In the above we want to group items according to the same kind.
E.g. include's with include's , let's with let's, type's with type's etc.
@chenglou pointed it would better for the readability to force a newline above
things that span over multiple lines. (handleClick and render for example)

I agree this is much more readable than

module Page = {
  include ReactRe.Component;
  
  type props = {message: string};
  type state = {count: int};
  
  let format = "a4";
  let name = "Page";
  let handleClick {props} event => {
    Js.log "clicked!";
    None
  };
  let render {props, updater} =>
    <div onClick=(updater handleClick)> (ReactRe.stringToElement props.message) </div>;
};

Another thing is we might need 'normalisation` of newlines above 'groups'. Example:

module page = {
  include ReactRe.Component;
  
  /* 
      comments get a newline above, but since this is the first item of a group
      we already have a newline. 1 + 1 = 2 ? This should be 'normalised` into one.
  */
  type props = {message: string};
  type state = {count: int};
}

If we don't do this, we get:

module page = {
  include ReactRe.Component;
  

  /* ^^ two newlines above */
  type props = {message: string};
  type state = {count: int};
}

Doesn't look that great.

I would propose something like:

type showNewLines = 
   |  IfBreak /* when a node spans over multiple lines */
   | Always
   | Never

makeList ~newLinesAbove:1 ~showNewLines:IfBreak ~normalizeNewLines:true []

Gonna dive into EasyFormat to see if this is possible.

@anmonteiro anmonteiro closed this as not planned Won't fix, can't repro, duplicate, stale Jul 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants