Swift4 Fix #48 #49

Ponyboy47 · 2017-07-08T18:39:40Z

Removed unnecessary imports
Removed _FMWrapper class and replaced with the default FileManager
Implemented subscripting using new Partial Ranges
Re-Implemented |>> using the TextFileStreamWriter
Replaced NSString and NSData in places where String or Data would also work
Added compiler flags for macOS as well as OSX, because I suspect the OSX flag will be deprecated at some point
Minor work towards including Linux support
Removed the _FMWrapper tests
Updated swift version and FileKit version

1) Removed unnecessary imports 2) Removed _FMWrapper class and replaced with the default FileManager 3) Implemented subscripting using new Partial Ranges 4) Re-Implemented `|>>` using the TextFileStreamWriter 5) Replaced NSString and NSData in places where String or Data would also work 6) Added compiler flags for macOS as well as OSX, because I suspect the OSX flag will be deprecated at some point 7) Minor work towards including Linux support 8) Removed the _FMWrapper tests 9) Updated swift version and FileKit version

… && added a rename function

nvzqz · 2017-07-08T19:05:17Z

Awesome! I'll take a thorough look at this tonight.

…ift4

…et _ = try?... instead Refactored if statements that returned early or threw errors to use guard in order to follow proper swift coding techniques

Ponyboy47 · 2017-07-11T20:04:43Z

Sorry that I keep adding commits. I've been slowly refactoring stuff as I go

nvzqz

I personally prefer using '///' for doc comments over block comments. Is there a reason for this change aside from it being your own preference?

nvzqz · 2017-07-08T18:52:27Z

FileKit.xcodeproj/project.pbxproj

@@ -917,7 +921,7 @@
 				PRODUCT_NAME = "$(PROJECT_NAME)";
 				SKIP_INSTALL = YES;
 				SWIFT_OPTIMIZATION_LEVEL = "-Owholemodule";
-				SWIFT_VERSION = 3.0;
+				SWIFT_VERSION = 4.0;


I've noticed that changing the Swift version in Xcode from 3 to 4 prevents projects from compiling with Xcode 8. Until Xcode 9 is officially released and replaces Xcode 8, the Swift version in Xcode should be 3.

nvzqz

Does this commit affect Swift 4 compatibility? If not, it should probably be in its own PR.

nvzqz · 2017-07-16T18:49:02Z

@Ponyboy47 I've noticed that changing the Swift version in Xcode from 3 to 4 prevents projects from compiling with Xcode 8. Xcode 9 is in its third beta. When Xcode 9 hits GM or the macOS App Store, then the Swift version project setting should change to accommodate the release. Until then, it should be 3.

phimage · 2017-08-26T05:38:16Z

Sources/ArrayFile.swift

+ A representation of a filesystem array file.
+
+ The data type is NSArray.
+*/
 public typealias NSArrayFile = File<NSArray>


Is it a swift4 needs to change all the comments?
If not, a separated PR would have been better

to see only swift 4 change

to discuss elsewhere about comment formatting

to merge it before merging swift 4

Swift 4 does not require a change in comment style. I've updated RandomKit to Swift 4 and much has remained the same as before, including the comment style.

phimage · 2017-08-26T05:42:42Z

Sources/NSString+FileKit.swift

                           atomically: useAuxiliaryFile,
                           encoding: String.Encoding.utf8.rawValue)
-        } catch {
+        else {


This code do not allow to debug using a breakpoint to see the error.
Swift4 is a breaking change so I think FileKitError.writeToFileFail must contain the error as second argument

phimage · 2017-08-26T05:47:19Z

Sources/Path.swift

            options: options)
        return (volumes ?? []).flatMap { Path(url: $0) }
    }

    // MARK: - Properties

-    fileprivate var _fmWraper = _FMWrapper()


Using a custom FileManager is a useful macOS feature, not really used for iOS
When you have a lot to do with files, you want to separate your FileManagerDelegate codes

phimage · 2017-08-26T05:52:29Z

Sources/Path.swift

    public var components: [Path] {
-        if rawValue == "" || rawValue == "." {
+        guard rawValue != "" && rawValue != "." else {


guard is not always the solution

I see a lot if replaced by guard with tricky boolean conditions

it's a matter of coding style here, if has been clear here, guard also is clear

phimage · 2017-08-26T05:56:14Z

Sources/TextFile.swift

+            return false
+        }
+        if delim && append {
+            handle.write(delimData)


This is a behaviour change
if append mode, you want the delim only before?
This change must be documented carefully(method comment, changelog when releasing) or be reverted

phimage · 2017-09-30T20:35:34Z

Thanks @Ponyboy47. As you can see your PR could not be merged.
I convert myself to not have all comments modified.

I do some more work because of your PR like "Re-Implemented |>> using the TextFileStreamWriter"
I do not do removing import or linux stuff.
You can PR for that later

Ponyboy47 added 2 commits July 8, 2017 12:33

Updated multiline (3+ lines) doc comments to the block comment format…

eb034f9

… && added a rename function

Ponyboy47 mentioned this pull request Jul 8, 2017

Support Swift 4 #48

Closed

Ponyboy47 added 4 commits July 10, 2017 13:32

Added vim swap files and swift Package.pins to the gitignore

67f999b

Updated to recomended xcode settings

fffec4d

Merge branch 'Swift4' of https://github.com/Ponyboy47/FileKit into Sw…

cd0dacf

…ift4

do/catch just to throw is strange behavior. Refactored to use guard l…

b6fae9a

…et _ = try?... instead Refactored if statements that returned early or threw errors to use guard in order to follow proper swift coding techniques

nvzqz reviewed Jul 16, 2017

View reviewed changes

phimage mentioned this pull request Aug 14, 2017

warning: 'ExpressibleByStringInterpolation' is deprecated #47

Open

phimage changed the title ~~Swift4~~ Swift4 Fix #48 Aug 14, 2017

phimage mentioned this pull request Aug 26, 2017

Support Decodable/Encodable (Swift 4) #50

Closed

4 tasks

phimage requested changes Aug 26, 2017

View reviewed changes

phimage closed this Sep 30, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Swift4 Fix #48 #49

Swift4 Fix #48 #49

Ponyboy47 commented Jul 8, 2017

nvzqz commented Jul 8, 2017

Ponyboy47 commented Jul 11, 2017

nvzqz left a comment

nvzqz Jul 8, 2017

nvzqz left a comment •

edited

Loading

nvzqz commented Jul 16, 2017 •

edited

Loading

phimage Aug 26, 2017 •

edited

Loading

nvzqz Aug 26, 2017

phimage Aug 26, 2017

phimage Aug 26, 2017

phimage Aug 26, 2017 •

edited

Loading

phimage Aug 26, 2017

phimage commented Sep 30, 2017

Swift4 Fix #48 #49

Swift4 Fix #48 #49

Conversation

Ponyboy47 commented Jul 8, 2017

nvzqz commented Jul 8, 2017

Ponyboy47 commented Jul 11, 2017

nvzqz left a comment

Choose a reason for hiding this comment

nvzqz Jul 8, 2017

Choose a reason for hiding this comment

nvzqz left a comment • edited Loading

Choose a reason for hiding this comment

nvzqz commented Jul 16, 2017 • edited Loading

phimage Aug 26, 2017 • edited Loading

Choose a reason for hiding this comment

nvzqz Aug 26, 2017

Choose a reason for hiding this comment

phimage Aug 26, 2017

Choose a reason for hiding this comment

phimage Aug 26, 2017

Choose a reason for hiding this comment

phimage Aug 26, 2017 • edited Loading

Choose a reason for hiding this comment

phimage Aug 26, 2017

Choose a reason for hiding this comment

phimage commented Sep 30, 2017

nvzqz left a comment •

edited

Loading

nvzqz commented Jul 16, 2017 •

edited

Loading

phimage Aug 26, 2017 •

edited

Loading

phimage Aug 26, 2017 •

edited

Loading