Fixed substring API warnings for Swift 3.2 and up
#2240
Conversation
|
thanks |
Source/Request.swift
Outdated
| components.append("-b \"\(string.substring(to: string.characters.index(before: string.endIndex)))\"") | ||
|
|
||
| #if swift(>=3.2) | ||
| components.append("-b \"\(string[..<string.index(before: string.endIndex)])\"") |
There was a problem hiding this comment.
I don't believe you need the before: call anymore, as ..< does essentially the same thing.
There was a problem hiding this comment.
I think we will still want the before: call. substring(to:) gets you every character except for the one specified so in this case it seems to be stripping out the last character in the string.
There was a problem hiding this comment.
index(before: endIndex gives you endIndex - 1. ..<endIndex also gives you endIndex - 1. ..<index(before: endIndex) would give you endIndex - 2. So to be equivalent to what it was before it would be ..<endIndex, no?
There was a problem hiding this comment.
The original version with substring(to:) also gives you endIndex - 2: -1 from index(before:) and another -1 from substring(to:).
Source/Request.swift
Outdated
| @@ -308,7 +308,12 @@ extension Request: CustomDebugStringConvertible { | |||
| let cookies = cookieStorage.cookies(for: url), !cookies.isEmpty | |||
| { | |||
| let string = cookies.reduce("") { $0 + "\($1.name)=\($1.value);" } | |||
| components.append("-b \"\(string.substring(to: string.characters.index(before: string.endIndex)))\"") | |||
|
|
|||
There was a problem hiding this comment.
No need for extra whitespace around the #if checks. We prefer to keep the whitespace the same as if there was no check in the first place.
Source/Request.swift
Outdated
| components.append("-b \"\(string.substring(to: string.characters.index(before: string.endIndex)))\"") | ||
|
|
||
| #if swift(>=3.2) | ||
| components.append("-b \"\(string[..<string.index(before: string.endIndex)])\"") |
There was a problem hiding this comment.
Please unindent the code inside the #if check to match out style.
Source/Request.swift
Outdated
| @@ -308,7 +308,12 @@ extension Request: CustomDebugStringConvertible { | |||
| let cookies = cookieStorage.cookies(for: url), !cookies.isEmpty | |||
| { | |||
| let string = cookies.reduce("") { $0 + "\($1.name)=\($1.value);" } | |||
|
|
|||
| #if swift(>=3.2) | |||
There was a problem hiding this comment.
#if statements and the code should be at the same level.
htinlinn
force-pushed
the
xcode-9-string-warnings
branch
from
a92d920 to
ceed9c4
Compare
Source/Request.swift
Outdated
| @@ -308,7 +308,12 @@ extension Request: CustomDebugStringConvertible { | |||
| let cookies = cookieStorage.cookies(for: url), !cookies.isEmpty | |||
| { | |||
| let string = cookies.reduce("") { $0 + "\($1.name)=\($1.value);" } | |||
|
|
|||
There was a problem hiding this comment.
Sorry, like I said before, please remove the newlines before and after all version checks. Basically, where there are version checks, the whitespace should match what was there before. Same indentation level, same newlines.
|
Hi @htinlinn, The compiler warnings you mention here do not appear for me in Xcode 9 beta 5. I'm currently installing beta 6 and will give it a shot there, but I don't anticipate we'll see the warnings there either. Were you seeing these issues in beta 4? I'm going to close issue out for now since I'm not seeing the warnings. We'll certainly take a pass at updating all the APIs and implementations to the latest Swift 4 syntax in the AF 5 branch.
Thanks anyways! 🍻 |
htinlinn
force-pushed
the
xcode-9-string-warnings
branch
from
ceed9c4 to
d59687e
Compare
|
@jshier Done. Sorry about all the back and forth with this. I wasn't thinking about newlines when you said whitespace. |
|
Ah thanks @jshier...late night. I figured I was missing something. 😕 |
|
Merging without full Travis, because it would likely be hours before Travis got to us. All tests pass locally for me. |
|
Looks like we've still got three substring deprecated and one 'characters' deprecated warnings hanging around. Unless the version I'm running from cocoapads isn't up to date? |
As of the latest version of Xcode 9 beta,
substringfunctions fromStringhave been deprecated and show up as warnings when compiling using Swift 4 compiler.This pull request fixes those warnings by replacing calls to those functions with the new subscript slicing syntax.