Matt — thanks a ton for all the detailed comments! Just quickly I figured out the git/hub steps to split out the top-level repositories into separate “ki” and “gi” repos, so the link and package paths are now:
https://github.com/goki/gi and ki and hopefully this now works for the demo — just worked for me on my mac.. > go get github.com/goki/gi > cd ~/go/src/github.com/goki/gi/examples/widgets > go get ... > go build > ./widgets More reactions later but just wanted to get that done so the paths should be stable now! Cheers, - Randy > On May 4, 2018, at 9:09 AM, matthewju...@gmail.com wrote: > > Hi Randy, here’s a code review. > > Thanks for the BSD license. > > I prefer the look of a minimized import path, I would have put the title > library at the top level (github.com/goki/ki). > > To me the README doesn’t balance text and code examples well enough, I’d like > to see more example uses and less text. In package ki I’d hope the library > does enough work to not require so much README. > > I think ki isn’t a bad package name after reading what it means but seeing > the package used in app code won’t show the point as obviously as an English > word for some people. tree.New() to me says “new data structure var” while > ki.New() says “I have to read the docs now”. > > (when I say 'app' I mean a program that uses your library or a program > written to compile with a Go compiler; an application of the library, an > application of the Go programming language) > > The Ki interface is a code smell to me. Usually I interpret library > interfaces as saying “the app provides an implementation of the interface and > the library provides shared logic that uses the interface methods” or in this > case I expect the ki lib will provide varying data structures and behaviors > that implement the ki var. Just reading down to line 42 of ki.go I feel like > this isn’t very Go-like. > > You’re getting into generics territory with this: > > func NewOfType(typ reflect.Type) Ki { > > My view is idiomatic Go code tends to avoid this kind of thing due to > maintenance and readability burden. I haven’t used your lib but I am > concerned about it not being a big win over a per-app implementation. I can > see you’ve done a lot of work to make generics work. > > In type Node I would consider embedding Props. > > I haven’t looked at all of the methods but do the methods on Node need to be > to a pointer? > > func (n *Node) Fields() []uintptr { > > Seeing uintptr in a public method is a code smell to me. > > In type Deleted consider maybe embedding sync.Mutex. > > // fmt.Printf("finding path: %v\n", k.Path) > > Instead of comments you may want to consider this pattern: > > const debug = false > … > if debug { > fmt.Printf("finding path: %v\n", k.Path) > } > > I think this library would be a good study for the Go 2 > (https://blog.golang.org/toward-go2) generics effort, if you have time > consider writing an experience report and posting it: > https://github.com/golang/go/issues/15292 > > Perhaps consider embedding ki.Signal in gi.Action, and MakeMenuFunc, > ki.Signal, *Icon, and ButtonStates in ButtonBase, some fields in ColorView, > Dialog, FillStyle, FontStyle, LayoutStyle, LayoutData, Layout, MapView, > MapViewInline, Node2DBase, Paint, ImagePaintServer, SliceView, > SliceViewInline, SliderBase, SplitView, StrokeStyle, StructView, > StructViewInline, BorderStyle, ShadowStyle, Style, TabView, TextStyle, > TextField, SpinBox, ComboBox, TreeView, ValueViewBase, Viewport2D, Window. I > like to avoid any unnecessary field symbols. > > These kinds of structs are a code smell to me: > > type ColorValueView struct { > ValueViewBase > } > > Why a Base type? Can your types be simpler? > > type Node2D and type ValueView seem like other possible overuses of interface > to me. The size of type PaintServer is closer to what I’d expect with an > interface, and Labeler seems idiomatic. > > I like the screenshot. > > // check for interface implementation > var _ Node2D = &Line{} > > I don’t like this pattern. > > Given the amount of code and project maturity I wouldn’t expect you to make a > lot of changes, but I do think it could have been built with a stronger > resilience to change. Thanks for sharing here. > > Matt > > On Friday, May 4, 2018 at 5:39:35 AM UTC-5, Randall O'Reilly wrote: > https://github.com/goki/goki — key demo in: > https://github.com/goki/goki/tree/master/gi/examples/widgets > > This is the first release of a new Go framework built around the Tree as a > core data structure (Ki = Tree in Japanese), which includes as its first > application a fully-native Go GUI (built on top of a modified version of the > Shiny OS-specific backend drivers, supporting Mac, Linux, and Windows so > far). > > Building on the central idea in Go that having a few powerful data-structures > is essential for making many problems easier to solve, the GoKi trees are an > attempt to provide a powerful tree structure that can support things like > scene graphs, DOM’s, parsing trees, etc. > > The GoGi graphical interface system is a kind of “proof is in the pudding” > test, which weighs in at under 20k LOC and provides a reasonably > full-featured GUI — with a bit more work it should be able to do most of the > stuff you can do in Qt, and already includes a (self) reflection-driven GUI > designer. > > The overall design is an attempt to integrate existing standards and > conventions from widely-used frameworks, including Qt (overall widget > design), HTML / CSS (styling), and SVG (rendering). Rendering in SVG is > directly supported by the GoGi 2D scenegraph, with enhanced functionality for > interactive GUI's. This 2D framework also integrates with a (planned) 3D > scenegraph, to support interesting combinations of these frameworks. > Currently GoGi is focused on desktop systems, but nothing prevents adaptation > to mobile. > > Right now the rendering is based off of a modified version of > https://github.com/fogleman/gg, but I’m very interested in integrating the > new rasterx system that Steven Wiley recently announced. > > I’d be very interested in people’s impressions, suggestions, etc, and welcome > all interested contributors (there’s certainly much more to do) — it would be > great if this could provide the start for a widely-supported Go-native GUI > framework! This was my first Go project after many years in C++ / Qt land, > and I’m excited to join the community, and have really been impressed with > the language and ecosystem etc. The contrast in complexity and build time > between Qt and GoGi is really striking, and has kept me going despite the > huge amount of effort it took to get this new project off the ground.. > Cheers, > > - Randy > > > -- > You received this message because you are subscribed to the Google Groups > "golang-nuts" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to golang-nuts+unsubscr...@googlegroups.com. > For more options, visit https://groups.google.com/d/optout. -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
