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

Update for Swift 4.0 #228

Merged
merged 14 commits into from
Sep 25, 2017
Merged

Update for Swift 4.0 #228

merged 14 commits into from
Sep 25, 2017

Conversation

paulw11
Copy link
Contributor

@paulw11 paulw11 commented Sep 21, 2017

No description provided.

@p2
Copy link
Owner

p2 commented Sep 21, 2017

Cool, thanks! Can you please also update the .swift-version and .travis.yml files, and use tabs instead of spaces in the code files?

Copy link
Owner

@p2 p2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks - still some leftover spaces. Easiest to catch when looking at the diff on Github.

@@ -471,7 +471,7 @@ open class OAuth2ContextStore {
open var state: String {
if _state.isEmpty {
_state = UUID().uuidString
_state = _state[_state.startIndex..<_state.index(_state.startIndex, offsetBy: 8)] // only use the first 8 chars, should be enough
_state = String(_state[_state.startIndex..<_state.index(_state.startIndex, offsetBy: 8)]) // only use the first 8 chars, should be enough
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces

@@ -20,7 +20,7 @@

import Foundation
#if !NO_MODULE_IMPORT
import Base
import Base
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces

@@ -167,11 +167,11 @@ open class OAuth2WebViewController: UIViewController, WKNavigationDelegate {
let _ = webView?.load(URLRequest(url: url))
}

func goBack(_ sender: AnyObject?) {
@objc func goBack(_ sender: AnyObject?) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces

let _ = webView?.goBack()
}

func cancel(_ sender: AnyObject?) {
@objc func cancel(_ sender: AnyObject?) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces

@@ -216,7 +216,7 @@ public class OAuth2WebViewController: NSViewController, WKNavigationDelegate, NS
/**
Tells the web view to stop loading the current page, then calls the `onWillCancel` block if it has a value.
*/
func cancel(_ sender: AnyObject?) {
@objc func cancel(_ sender: AnyObject?) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still spaces.

@@ -280,8 +280,8 @@ public class OAuth2WebViewController: NSViewController, WKNavigationDelegate, NS


// MARK: - Window Delegate

public func windowShouldClose(_ sender: Any) -> Bool {

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still spaces.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still spaces.

@paulw11
Copy link
Contributor Author

paulw11 commented Sep 21, 2017

Yeah, I am still working through it; I had to go do some other things.

I actually didn't touch most of that by hand. Perhaps the Xcode "fix this for Swift 4" has put spaces in.

I did change the lines near the #if because there is a bug in Xcode when it compiles for the migration tool it doesn't honour any -D in the build config, so I was trying to work around that. In the end I just ignored the migration tool.

@p2
Copy link
Owner

p2 commented Sep 22, 2017

Yeah sure! Xcode doesn't handle this well, until you switch to tabs in prefs it won't do the right thing. Let me know when you're done and I'll take another look!

@paulw11
Copy link
Contributor Author

paulw11 commented Sep 22, 2017 via email

Copy link
Owner

@p2 p2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still spaces left in some places. You can see them easily on Github.

_state = String(_state[_state.startIndex..<_state.index(_state.startIndex, offsetBy: 8)]) // only use the first 8 chars, should be enough
}
return _state
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces.

@@ -216,7 +216,7 @@ public class OAuth2WebViewController: NSViewController, WKNavigationDelegate, NS
/**
Tells the web view to stop loading the current page, then calls the `onWillCancel` block if it has a value.
*/
func cancel(_ sender: AnyObject?) {
@objc func cancel(_ sender: AnyObject?) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still spaces.

@@ -280,8 +280,8 @@ public class OAuth2WebViewController: NSViewController, WKNavigationDelegate, NS


// MARK: - Window Delegate

public func windowShouldClose(_ sender: Any) -> Bool {

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still spaces.

Copy link
Owner

@p2 p2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please change to tabs in Xcode, visit all the places that show up on Github, and ensure there are no spaces in indents any more.

@@ -468,12 +468,13 @@ open class OAuth2ContextStore {

We internally generate a UUID and use the first 8 chars if `_state` is empty.
*/
open var state: String {
open var state: String {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces.

if _state.isEmpty {
_state = UUID().uuidString
_state = _state[_state.startIndex..<_state.index(_state.startIndex, offsetBy: 8)] // only use the first 8 chars, should be enough
_state = UUID().uuidString
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spaces.

}
return _state

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extra newline.

@@ -280,8 +280,8 @@ public class OAuth2WebViewController: NSViewController, WKNavigationDelegate, NS


// MARK: - Window Delegate

public func windowShouldClose(_ sender: Any) -> Bool {

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still spaces.

@p2
Copy link
Owner

p2 commented Sep 23, 2017

(Those spaces are on the newline before public func)

@paulw11
Copy link
Contributor Author

paulw11 commented Sep 23, 2017

Copy link
Owner

@p2 p2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LOL, yeah, I saw that. Guess I'll never get rich!

In your last commit you removed the spaces but forgot to add a tab for indent. And you forgot to add yourself to CONTRIBUTORS.md! When that's gone we'll finally be able to merge. 👍🏼

@@ -280,8 +280,8 @@ public class OAuth2WebViewController: NSViewController, WKNavigationDelegate, NS


// MARK: - Window Delegate

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now the indent is gone. 😁

Copy link
Owner

@p2 p2 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please check Sources/macOS/OAuth2WebViewController.swift:

  • Line 283: should have one tab, currently line is empty
  • Line 284: should have one tab before public func, currently has 4 spaces

@p2 p2 merged commit e630ac7 into p2:master Sep 25, 2017
@ksteigerwald
Copy link

When will the podspec be updated?

@p2 p2 mentioned this pull request Nov 9, 2017
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.

3 participants