xi-editor / xi-mac

The xi-editor mac frontend.
Apache License 2.0
3.02k stars 147 forks source link

JSON Serialization: Parse responses fully #444

Closed jeremywiebe closed 5 years ago

jeremywiebe commented 5 years ago

Summary

This PR explores managing JSON (de)serialization a bit more cleanly. It pushes all parsing as close to message reception as possible. We already have some of the responses from core being parsed, but in many cases the deeper we get into the JSON structure, the more likely we just punt a [String: AnyObject] out to the rest of the codebase.

My thinking is that we'd never let any code other than the RPCSending know about the JSON structures coming from core. They'd all be parsed into strongly-typed structs or enums.

Related Issues

This is somewhat related to https://github.com/xi-editor/xi-mac/issues/102. However, I don't see this adding too much in the way of performance. I see this as a way to make the xi-mac codebase cleaner and less fragile.

Checklist

Review Checklist

nangtrongvuon commented 5 years ago

It's very great to have you contributing this to xi. I look forward to learning from your work @jeremywiebe !

jeremywiebe commented 5 years ago

Oh good catch! Thanks for pointing that out.

On Tue, 19 Mar 2019 at 21:43, Ryosei Shinoki notifications@github.com wrote:

@rshinoki commented on this pull request.

In Sources/XiEditor/Core/Types.swift https://github.com/xi-editor/xi-mac/pull/444#discussion_r267184026:

+// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +import Foundation + +enum UpdateOperationType: String {

  • typealias RawValue = String
  • case Copy = "copy"
  • case Invalidate = "invalidate"
  • case Insert = "ins"
  • case Update = "update"
  • case Skip = "skip"

The coding style of the enum case should be lower camel case. In Swift guideline and other companies' coding style, enum case is lower camel case. https://docs.swift.org/swift-book/LanguageGuide/Enumerations.html

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/xi-editor/xi-mac/pull/444#pullrequestreview-216525948, or mute the thread https://github.com/notifications/unsubscribe-auth/AAEtUlhZ5ADuCFfNIjaScWODe8kbhQ-Oks5vYbyKgaJpZM4bhRV2 .

--

Jeremy Wiebe jeremy.wiebe@gmail.com

nangtrongvuon commented 5 years ago

@jeremywiebe Let me know if you want a review, as I might be doing stuff which could introduce some more work for you :)

jeremywiebe commented 5 years ago

@nangtrongvuon I don’t feel like I’m fully settled on how I’m going to be organizing everything yet. But feel free to post comments on what you see so far. Based on what you’re planning it might change how I end up doing this. :-)

cmyr commented 5 years ago

I think this looks good, thanks a bunch @jeremywiebe for taking this on. @mmatoszko happy to have you take a closer look this week; I'll do another final pass when you're satisfied, but I don't expect to have any issues.

jeremywiebe commented 5 years ago

Thanks for the review @cmyr and @mmatoszko!