upload a gcode file received via iOS/iPadOS share feature#598
upload a gcode file received via iOS/iPadOS share feature#598p3d-dev wants to merge 2 commits intogdombiak:masterfrom
Conversation
|
In the meantime the share issue on slicer side is fixed. Sharing to octopod works like a breeze now. |
gdombiak
left a comment
gdombiak
left a comment
There was a problem hiding this comment.
Thanks for the PR. It looks very good. I haven't tested it locally but left some comments before taking it for a local spin/test and merge.
Regards,
Gaston
| imageView.image = UIImage(named: file.isModel() ? "Model_48" : "GCode_48") | ||
| if let thumbnailURL = file.thumbnail { | ||
| octoprintClient.getThumbnailImage(path: thumbnailURL) { (data: Data?, error: Error?, response: HTTPURLResponse) in | ||
| octoprintClient.getThumbnailImage(path: thumbnailURL) { (data: Data?, _: Error?, _: HTTPURLResponse) in |
There was a problem hiding this comment.
As coding style I like to be explicit about parameters for easier maintenance and readability. Would you be ok reverting this change for both params? Thanks
There was a problem hiding this comment.
Reverting is ok, but then the compiler complains with a warning. Any idea, how to resolve this ?
| func tableView(_ tableView: UITableView, trailingSwipeActionsConfigurationForRowAt indexPath: IndexPath) -> UISwipeActionsConfiguration? { | ||
| let deleteAction = UIContextualAction(style: .destructive, title: NSLocalizedString("Delete", comment: "Delete action"), handler: { (action, view, completionHandler) in | ||
| self.showConfirm(message: NSLocalizedString("Do you want to delete this file?", comment: "")) { (UIAlertAction) in | ||
| let deleteAction = UIContextualAction(style: .destructive, title: NSLocalizedString("Delete", comment: "Delete action"), handler: { _, _, completionHandler in |
There was a problem hiding this comment.
same here (explicit param name)
There was a problem hiding this comment.
Reverting is ok, but then the compiler complains with a warning. Any idea, how to resolve this ?
| let deleteAction = UIContextualAction(style: .destructive, title: NSLocalizedString("Delete", comment: "Delete action"), handler: { (action, view, completionHandler) in | ||
| self.showConfirm(message: NSLocalizedString("Do you want to delete this file?", comment: "")) { (UIAlertAction) in | ||
| let deleteAction = UIContextualAction(style: .destructive, title: NSLocalizedString("Delete", comment: "Delete action"), handler: { _, _, completionHandler in | ||
| self.showConfirm(message: NSLocalizedString("Do you want to delete this file?", comment: "")) { _ in |
There was a problem hiding this comment.
same here (explicit param name)
There was a problem hiding this comment.
Reverting is ok, but then the compiler complains with a warning. Any idea, how to resolve this ?
| self.tableView.reloadData() | ||
| completionHandler(true) | ||
| } no: { (UIAlertAction) in | ||
| } no: { _ in |
There was a problem hiding this comment.
same here (explicit param name)
There was a problem hiding this comment.
Reverting is ok, but then the compiler complains with a warning. Any idea, how to resolve this ?
| // Initialize SD card if needed and refresh files from SD card | ||
| @IBAction func refreshSDCard(_ sender: Any) { | ||
| octoprintClient.refreshSD { (success: Bool, error: Error?, response: HTTPURLResponse) in | ||
| octoprintClient.refreshSD { (success: Bool, _: Error?, response: HTTPURLResponse) in |
There was a problem hiding this comment.
same here (explicit param name)
| } else if response.statusCode == 409 { | ||
| // SD Card is not initialized so initialize it now | ||
| self.octoprintClient.initSD(callback: { (success: Bool, error: Error?, response: HTTPURLResponse) in | ||
| self.octoprintClient.initSD(callback: { (success: Bool, _: Error?, _: HTTPURLResponse) in |
There was a problem hiding this comment.
same here (explicit param name)
There was a problem hiding this comment.
Reverting is ok, but then the compiler complains with a warning. Any idea, how to resolve this ?
| // Load all files and folders (recursive) | ||
| octoprintClient.files { (result: NSObject?, error: Error?, response: HTTPURLResponse) in | ||
| var newFiles: Array<PrintFile> = Array() | ||
| octoprintClient.files { (result: NSObject?, error: Error?, _: HTTPURLResponse) in |
There was a problem hiding this comment.
same here (explicit param name)
There was a problem hiding this comment.
Reverting is ok, but then the compiler complains with a warning. Any idea, how to resolve this ?
|
|
||
| // Delete from server (if failed then show error message and reload) | ||
| octoprintClient.deleteFile(origin: printFile.origin!, path: printFile.path!) { (success: Bool, error: Error?, response: HTTPURLResponse) in | ||
| octoprintClient.deleteFile(origin: printFile.origin!, path: printFile.path!) { (success: Bool, _: Error?, response: HTTPURLResponse) in |
There was a problem hiding this comment.
same here (explicit param name)
There was a problem hiding this comment.
Reverting is ok, but then the compiler complains with a warning. Any idea, how to resolve this ?
| searchedFiles.remove(at: indexPath.row) | ||
| files.removeAll { (someFile: PrintFile) -> Bool in | ||
| return someFile == printFile | ||
| someFile == printFile |
There was a problem hiding this comment.
was the removal of the 'return' keyword intentional?
There was a problem hiding this comment.
actually this has been done by SwiftFormat, because the return is not needed in this special case
|
Apparently all the issues are due to coding styles. If a format ing tool is used, then endless discussions about different flavors can be avoided. SwiftFormat is nicely integrated in Xcode, that‘s why it was applied to the changed files. On Linux, swift-format is an alternative. If you prefer to have explicit parameter names, then how to get rid of the compiler warnings ? Just keep explicit parameter names and tolerating compiler warnings, appears not to be a good solution. Anyway, whatever coding style you like to apply in your project is up to you. So all those changes proposed can be applied. |
|
Thanks for taking the time to review the PR. |
3 participants
In reference to #597: This PR presents a quick implementation for the share feature to quickly upload a gcode file. This works in these steps:
From the slicer, it does not work yet, but this may be slicer related or still some wrong definitions. For sure the whole UTTypeIdentifier/Document/Imported Type Identifiers is not self-explaining. So there may be issues on either side.
Two additional notes:
Please comment