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

Batch smie fixes #76

Merged
merged 48 commits into from Feb 23, 2015
Merged

Batch smie fixes #76

merged 48 commits into from Feb 23, 2015

Conversation

ap4y
Copy link
Collaborator

@ap4y ap4y commented Jan 26, 2015

Batch of improvements for the issues raised in #74. Also fixes #60, #73, #58, #77, #80, #81.

ap4y added 28 commits January 23, 2015 22:03
It's important in conjuctiontions with case statement. It's a common
rule to connect ":" with a variable. In this case ":" will appear as a
part of the identifier, we need this token separately to acivate grammar.
is a last or first symbol on the line. Important for ternary operator.
swift code) and improve support for several form of multi-line
expressions
@ap4y ap4y mentioned this pull request Jan 26, 2015
@ap4y
Copy link
Collaborator Author

ap4y commented Jan 26, 2015

Don't have emacs 23 atm. Will check failing tests in VM tomorrow, will take some time.

@ap4y
Copy link
Collaborator Author

ap4y commented Feb 22, 2015

Hey @bbatsov or @taku0, can I ask you to review this PR. Sorry it has so many commits and it took so long, but it fixes quite a few issues raised by @taku0 and many others. It also provides alternative solution to what @taku0 did in #74. I didn't change cases that are qualified as personal preferences. I would prefer to address those cases in separate PR with possibility of adding customizable parameters. I hope I didn't miss any issues here, but if that happened feel free to create a new issue or re-open a previous one.

I tried to preserve support for the emacs 24.3, unfortunately it wasn't possible for 3 test cases. For some reason emacs 24.3 treats parent based indentation rules differently (I actually think it's a bug), so I had to make assertions conditional. This sounds a bit gross, so I'm keen to hear other opinions about what should we do in this case.

@bbatsov
Copy link
Collaborator

bbatsov commented Feb 22, 2015

I'll have a look at the code.

I tried to preserve support for the emacs 24.3, unfortunately it wasn't possible for 3 test cases. For some reason emacs 24.3 treats parent based indentation rules differently (I actually think it's a bug), so I had to make assertions conditional. This sounds a bit gross, so I'm keen to hear other opinions about what should we do in this case.

As many modes started to adopt smie in 24.4 a lot of bugs were fixed then. I'm fine with dropping 24.3 support altogether. As I said before - the upgrade path is trivial on all operating systems.

@taku0
Copy link
Member

taku0 commented Feb 22, 2015

It seems that there are almost no problems for practical use.

Some cases we should handle better:

let foo =
    bar +
  baz +
  a

enum Foo: Bar {
    case foo
    case bar
        var foo
}

let foo = [
    bar:
    baz, // I want an extra indentation for this line
    bar:
    baz, // and this line
]

enum Foo {
    case A(Int, [Int : String]), B, C
                                    func foo() {
    }
}

@Foo public class Foo {
         a
     }

@ap4y
Copy link
Collaborator Author

ap4y commented Feb 23, 2015

Thanks @bbatsov, I will update my PR today with support for 24.4 only.

Thanks @taku0 for checking PR once again, I would appreciate if you will create a separate issues for all the cases I missed or you think have to behave differently.

@ap4y
Copy link
Collaborator Author

ap4y commented Feb 23, 2015

@bbatsov this should be ready to merge.

bbatsov added a commit that referenced this pull request Feb 23, 2015
@bbatsov bbatsov merged commit 195d040 into swift-emacs:master Feb 23, 2015
@bbatsov
Copy link
Collaborator

bbatsov commented Feb 23, 2015

Yeah, everything looks good! Great work, @ap4y! You'll soon give XCode a run for its money! :-)

@ap4y
Copy link
Collaborator Author

ap4y commented Feb 23, 2015

Haha thanks :) XCode indentation engine is a complete disaster imo. I'm going to close some issues, github doesn't support list of references, so will do it manually.

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.

Indention bug: return statement with ternary operator
3 participants