Philipp Stanner: 1 all: Implement working hierarchical subproj print 5 files changed, 193 insertions(+), 36 deletions(-)
Copy & paste the following snippet into your terminal to import this patchset into git:
curl -s https://lists.sr.ht/~imperator/quartiermeister-devel/patches/20836/mbox | git am -3Learn more about email & git
Suproject printing now works. New problem: container & project are interwined too strongly. Should be fixed soonish. --- bin/qm-list/main.go | 116 ++++++++++++++++++++++++++++++++++++++------ lib/collection.go | 85 +++++++++++++++++++++++++++++--- lib/container.go | 20 ++++---- lib/project.go | 5 +- qm | 3 +- 5 files changed, 193 insertions(+), 36 deletions(-) diff --git a/bin/qm-list/main.go b/bin/qm-list/main.go index 512baca..e210538 100644 --- a/bin/qm-list/main.go @@ -2,51 +2,135 @@ package main import ( "fmt" + "os" qm "git.sr.ht/~imperator/quartiermeister/lib" "github.com/integrii/flaggy" ) -func listItems(c *qm.Collection) { +type optionsS struct { + recurse uint
Stefan Tatschner <stefan@rumpelsepp.org>optionsS? What does the S suffix mean?
+ noLines bool +} + +// options should be global, otherwise get passed around 1000 times. +var ( + opts optionsS + gDepth int // necessary for recursive function call. Better idea welcome. +)
Stefan Tatschner <stefan@rumpelsepp.org>What does the g prefix mean? Also, I don't like global variables in Go. The pollute the namespace as they are global per package. This makes a future refactoring quite a pain in the ass. Wrap them in a struct and use struct methods instead if required. Also, don't use recursion. Every recursive code can be written in an iterative way.
+ +func listItems(c *qm.Collection) error { items, err := c.AllItems() if err != nil { - fmt.Println("error: ", err) - return + return err } for _, i := range items { fmt.Println(i.Title) } + + return nil } -func listProjs(c *qm.Collection) { - projs, err := c.AllProjects() +func prefixByDepth(depth int) string { + if depth == 0 { + return "" + } + prefix := "├─" + lengthener := "────" + if opts.noLines {
Stefan Tatschner <stefan@rumpelsepp.org>style: var ( prefix = "---" lengthener = "------" ) What's a lenghtener? This variable name sounds confusing for me.
+ prefix = " " + lengthener = " " + } + + var longer string + for i := 0; i < depth; i++ { + longer += lengthener + } + prefix += longer + + return prefix +} + +// prints passed subproject and all its subprojects recursively +func printProjAndSubs(subproj *qm.ProjectSubTreeS) error { + var err error + + // when this function recurses, the recursors get higher and higher + // numbers, causing the right indentation. + prefix := prefixByDepth(gDepth) + gDepth += 1 + + // TODO: find out why there is a whitespace too much + fmt.Println(prefix, subproj.Proj.Title) + + for _, proj := range subproj.Subprojs { + prefix = prefixByDepth(gDepth) + fmt.Println(prefix, proj.Proj.Title) + subprojs, err := proj.Proj.GetSubprojectTree() + if err != nil { + goto out + }
Stefan Tatschner <stefan@rumpelsepp.org>Don't use goto in Go code. There is e.g. defer for this. goto in go idiomatically is used for breaking out of nested loops, but not for cleanups. Despite it works (and I know the pattern from C) it is very uncommon and strange in Go code. I stopped the review here, since I was overflowed with too much intermixed information. Can you create smaller commits for the review? S
+ + for _, sub := range subprojs { + gDepth += 1 // ugly... recursion. + printProjAndSubs(sub) + gDepth -= 1 + } + } + +out: + gDepth -= 1 + return err +} + +// loads all projects and passes them to the print function +func printProjFamily(c *qm.Collection) error { + projs, err := c.AllProjectsN(1) if err != nil { - fmt.Println("error: ", err) - return + return err } - for _, p := range projs { - fmt.Println(p.Title) + + for _, proj := range projs { + tmpSubs, err := proj.GetSubprojectTree() + if err != nil { + return err + } + + tmp := qm.ProjectSubTreeS{proj, tmpSubs} + if err = printProjAndSubs(&tmp); err != nil { + return err + } } + + return nil } func main() { - var ( - itemCmd = flaggy.NewSubcommand("items") - projCmd = flaggy.NewSubcommand("projs") - ) + var err error + itemCmd := flaggy.NewSubcommand("items") + projCmd := flaggy.NewSubcommand("projs") + projCmd.UInt(&opts.recurse, "r", "recurse", + "<N> show subprojects N layers deep") + flaggy.Bool(&opts.noLines, "n", "no-lines", "Show no lines") + flaggy.AttachSubcommand(itemCmd, 1) flaggy.AttachSubcommand(projCmd, 1) flaggy.Parse() c, err := qm.LoadCollection() if err != nil { - fmt.Println("error: ", err) + fmt.Fprintln(os.Stderr, "error: ", err) return } if itemCmd.Used { - listItems(c) + err = listItems(c) } else if projCmd.Used { - listProjs(c) + err = printProjFamily(c) + } + + if err != nil { + fmt.Fprintln(os.Stderr, err) + os.Exit(1) } } diff --git a/lib/collection.go b/lib/collection.go index 37784ff..6a28d35 100644 --- a/lib/collection.go +++ b/lib/collection.go @@ -23,6 +23,11 @@ const ( SearchAttrTag = "tag" ) +type ProjectSubTreeS struct { + Proj *Project + Subprojs []*ProjectSubTreeS +} + type Collection struct { Path string PoolPath string @@ -99,6 +104,7 @@ func getProjMeta(path string) (*Project, error) { return proj, nil } +// TODO: Works only properly with minimum number = 1. Maybe 0 is better? func buildLayerNGlob(depth uint) string { var i uint s := "/" @@ -112,34 +118,99 @@ func buildLayerNGlob(depth uint) string { } // get subprojects N layers deep -func (p *Project) GetSubprojectsN(depth uint) ([]*Project, error) { +func (p *Project) GetSubprojectsN(depth int) ([]*Project, error) { var subprojs []*Project - glob := p.Path + buildLayerNGlob(depth) - fmt.Println("glob: ", glob) + glob := p.Path + buildLayerNGlob(uint(depth)) matches, err := filepath.Glob(glob) if err != nil { return nil, err } - fmt.Println("nr of matches: ", len(matches)) for _, m := range matches { - p, err := getProjMeta(m) + foundP, err := getProjMeta(m) if err != nil { return nil, err } - subprojs = append(subprojs, p) + + if foundP.UUID != p.UUID { + subprojs = append(subprojs, foundP) + } } return subprojs, nil } +// calls itself recursively until all subprojects have been found. +func (p *Project) GetSubprojectTree() ([]*ProjectSubTreeS, error) { + subprojs, err := p.GetSubprojectsN(1) + if len(subprojs) == 0 || err != nil { + return nil, err + } + + subtree := make([]*ProjectSubTreeS, 0, 8) + + for _, subpr := range subprojs { + deeperSubs, err := subpr.GetSubprojectTree() + if err != nil { + break + } + filler := ProjectSubTreeS{subpr, deeperSubs} + subtree = append(subtree, &filler) + } + + return subtree, err +} + +// TODO implement this +func (c *Collection) GetProjectTree() ([]ProjectSubTreeS, error) { + var tree []ProjectSubTreeS + + _, err := c.AllProjects() + if err != nil { + return nil, err + } + + return tree, nil +} + +// Gets all Projects layer N deep. Can be used to get all root projects. +func (c *Collection) AllProjectsN(depth int) ([]*Project, error) { + var ( + projs []*Project + err error + ) + if depth == 0 { + return nil, fmt.Errorf("Invalid depth") + } else if depth == -1 { + return c.AllProjects() + } + + glob := c.ProjectsPath + buildLayerNGlob(uint(depth)) + matches, err := filepath.Glob(glob) + if err != nil { + return nil, err + } + + for _, m := range matches { + foundP, err := getProjMeta(m) + if err != nil { + return nil, err + } + + projs = append(projs, foundP) + } + + return projs, nil +} + func (c *Collection) AllProjects() ([]*Project, error) { var ( projs []*Project err error ) + // FIXME: Chdir sucks if err = os.Chdir(c.ProjectsPath); err != nil { return nil, err } @@ -208,6 +279,8 @@ func (p *Collection) FindTags(t *Item) ([]*Tag, error) { return out, nil } +// TODO: warn user when there is more than 1 proj with identical name +// if there is, than make him choose by UUID. func (c *Collection) ProjectByTitle(title string) (*Project, error) { // TODO: implement this more efficently projs, err := c.AllProjects() diff --git a/lib/container.go b/lib/container.go index 5859a42..7a32365 100644 --- a/lib/container.go +++ b/lib/container.go @@ -7,6 +7,7 @@ package qm import ( "encoding/json" + "github.com/google/uuid" "os" "path/filepath" "time" @@ -18,26 +19,24 @@ type Container struct { Description string `json:"description" yaml:"description"` CreatedAt time.Time `json:"created_at" yaml:"created_at"` Path string `json:"path" yaml:"-"` + UUID uuid.UUID `json:"uuid" yaml:"uuid"` parent string } -func checkParentExists(c *Collection, title string) error { - _, err := c.ProjectByTitle(title) - return err -} - +// FIXME: this should not only work for Projects func (c *Collection) CreateContainer(title, - descr, type_, parent string) (*Container, error) { + descr, type_, parentTitle string) (*Container, error) { var path string slug := Slug(title) - if parent == "" { + if parentTitle == "" { path = filepath.Join(c.Path, type_, slug) } else { - if err := checkParentExists(c, parent); err != nil { + parent, err := c.ProjectByTitle(parentTitle) + if err != nil { return nil, err } - path = filepath.Join(c.Path, type_, parent, slug) + path = filepath.Join(parent.Path, slug) } cont := &Container{ @@ -46,7 +45,8 @@ func (c *Collection) CreateContainer(title, Description: descr, CreatedAt: time.Now(), Path: path, - parent: parent, + UUID: uuid.New(), + parent: parentTitle, } return cont, nil diff --git a/lib/project.go b/lib/project.go index 946b3d0..51b33f1 100644 --- a/lib/project.go +++ b/lib/project.go @@ -19,10 +19,9 @@ type Project struct { //Progress uint `json:"progress" yaml:"progress"` } -func (c *Collection) CreateProject(title, description, - parent string) (*Project, error) { +func (c *Collection) CreateProject(title, descr, parent string) (*Project, error) { p := &Project{} - cont, err := c.CreateContainer(title, description, "projects", parent) + cont, err := c.CreateContainer(title, descr, "projects", parent) if err != nil { return nil, err } diff --git a/qm b/qm index a3b2d6f..5e50a0c 100755 --- a/qm +++ b/qm @@ -16,10 +16,11 @@ main() { export QM_EDITOR="nvim" if [[ ! -d "$QM_PATH" ]]; then - echo "\"$QM_PATH\" created." + echo "No QM database yet. Iniatlizing..." mkdir -p "$QM_PATH" mkdir -p "$QM_PATH"/$QM_COLLECTION/pool mkdir -p "$QM_PATH"/$QM_COLLECTION/projects + echo "\"$QM_PATH\" initialized." fi if which "qm-$cmd" >& /dev/null; then -- 2.20.1
Stefan Tatschner <stefan@rumpelsepp.org>Can you create separate, smaller commits? This is difficult to review. Also, I am not happy with the variable names here as there are too much unclear suffixes and prefixes for me. Just spend a few more letters.