Skip to content

Add custom var for allowing additional indent in function parameter lists #42

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

Closed
wants to merge 4 commits into from

Conversation

npc3
Copy link

@npc3 npc3 commented Jul 5, 2017

No description provided.

@josteink
Copy link
Member

josteink commented Jul 7, 2017

Hi there and thanks for the pull-request!

On initial glance it looks good. I especially appreciate the you adding such an extensive test-case for it.

I haven't had time to look through the code-changes in detail yet, and I'm going on vacation tomorrow so unfortunately I probably won't get this merged until I'm back.

I realize you've added a big test-document, which is good, but it would still be helpful for us if you could update the PR description to better illustrate what concrete changes this PR makes. Basically a short TS code-sample which shows indentation before and after the change.

That will definitely help us understand the context of the code-changes, and that will hopefully speed things up a little :)

Sounds good?

@npc3
Copy link
Author

npc3 commented Jul 13, 2017

Hi, basically the idea is to allow a configurable extra indent for function argument lists, similar to typescript-expr-indent-offset

e.g. right now, with (setq typescript-indent-level 2), you get

function functionName(
  param1,
  param2) {
  otherFunction(
    param1,
    param2);
}

With (setq typescript-fn-parameter-indent-offset 2) you can now get

function functionName(
    param1,
    param2) {
  otherFunction(
      param1,
      param2);
}

I should note that

function functionName(param1,
                      param2) {
  otherFunction(param1,
                param2);
}

will still be formatted the same regardless of the value of typescript-fn-parameter-indent-offset

The reasoning behind the change is purely selfish: this is what my company's style guide dictates.

@josteink
Copy link
Member

josteink commented Aug 1, 2017

OK. I'm back from my vacation and that explanation makes perfect sense. I have no problem accepting such a formatting-option.

Right now we have two issues though:

  1. Recent changes have introduced a conflict between master and your changes.
  2. And more pressingly, a tentative rewrite (to reuse as much of js-mode as possible) is taking place in PR 45.

While I'm sure 1 is easily resolved, I'm curious about what kind of impact this will have on 2. Does something like this already exist in js-mode? Is this a customization we will have to reinvent on top of js-mode, or is there an existing setting we can rely on there instead?

If you think we'll still need to maintain something like this in typescript-mode, even after a potential rewrite, feel free to resolve 1 and I'll do some testing and get this merged.

If you think this change will be made redundant by 2, we can keep this on hold and see how 2 turns out before closing.

Sounds good?

@josteink
Copy link
Member

josteink commented Aug 4, 2017

The outcome of the discussion in the other thread, I think the best approach would just to clear the merge conflict and we can get this one going :)

@josteink
Copy link
Member

Any news here? Like I said, if you're willing to clean up the merge conflicts, I'll be happy to commit this.

@josteink
Copy link
Member

josteink commented Oct 2, 2017

Closing due to lack of activity.

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.

2 participants