search

Jintin / Swimat

An Xcode formatter plug-in to format your swift code.
https://jintin.github.io/Swimat/
MIT License
1.65k stars 89 forks source link

Improve preferences management (#2) #170

Closed saagarjha closed 7 years ago

saagarjha commented 7 years ago

Share preferences across all targets, improve API consistency, and allow for "testing" configurations that are separate from the user's preferences

Jintin commented 7 years ago

The test is fail. could you take a look at it?

saagarjha commented 7 years ago

@Jintin Sorry, it looks like I lost a commit in the middle there. The tests are passing now.

saagarjha commented 7 years ago

@Jintin Test cases No.3 and No.8 are broken in Xcode 9 beta 4. Could you take a look at them?

Jintin commented 7 years ago

OK, I'll try it.

saagarjha commented 7 years ago

@Jintin Have you considered adding Swimat to the Swift Source Compatibility Suite? This would have probably caught the issue much earlier.

Jintin commented 7 years ago

Test cases No.3 and No.8 is fixed in latest commit.

Jintin commented 7 years ago

Strange thing is why travis not showing any error, would you like to check if travis is running our unitTest?

saagarjha commented 7 years ago

That's because I haven't changed this pull request since last week. The build is still using Xcode 9 beta 3, which doesn't have this problem (that's also why I haven't done anything yet, since it would break the build).

Jintin commented 7 years ago

There's still strange, it break origin Xcode test now. I'll take a look at it. XD

Jintin commented 7 years ago

in SwiftParser.swift retString.distance(from: newlineIndex, to: retString.endIndex) return different result due to different Xcode.... Is there any breaking change in swift 4 about the api???

saagarjha commented 7 years ago

Yes, that's what I'm saying. Between beta 3 and beta 4 something changed–I think this could have been caught by the compatibility suite, since I'm pretty sure they weren't intending to break anything.

saagarjha commented 7 years ago

@Jintin Addressed your concerns, and the tests pass now. 🙌 Do you want to merge it in?

Jintin commented 7 years ago

My previous fix is not correct anyway. I need find out why the result is different first than we know how to fix. For the swift-source-compat-suite, I think you can create another issue.

saagarjha commented 7 years ago

I do realize that, but IMO we should work on figuring this out later, especially since it's holding up half a dozen other pull requests. We can always go back and figure out what's going on–or file a bug report if necessary.

Jintin commented 7 years ago

Yes, but the modification I made is not correct. It break the xcode 8 build. Do you have any idea how to fix it? https://travis-ci.org/Jintin/Swimat/builds/259988985

saagarjha commented 7 years ago

Let me file a bug on the Swift JIRA. Could you reduce this to a short test case?

Jintin commented 7 years ago

Sure you can do that. Sorry I'm currently on a career move, might not have time until next week. Could you wait please or try to do that. All the ref is in the recent commit.

saagarjha commented 7 years ago

Sorry I'm currently on a career move, might not have time until next week.

Good luck to you! I'll see if I can figure it out this week; if not, I'll wait for you to come back.