Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

Dep status tree visualisation dot output #271

Merged
merged 14 commits into from Apr 13, 2017
27 changes: 24 additions & 3 deletions cmd/dep/graphviz.go
Expand Up @@ -5,10 +5,10 @@
package main

import (
"hash/fnv"
"strings"
"bytes"
"fmt"
"hash/fnv"
"strings"
)

type graphviz struct {
Expand Down Expand Up @@ -48,7 +48,7 @@ func (g graphviz) output() bytes.Buffer {
for _, dp := range g.ps {
for _, bsc := range dp.children {
for pr, hsh := range g.h {
if strings.HasPrefix(bsc, pr) {
if strings.HasPrefix(bsc, pr) && isPathPrefixOrEqual(pr, bsc) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was imagining this as a standalone implementation - move the strings.HasPrefix() call inside isPathPrefixOrEqual, rather than requiring two calls out here.

r := fmt.Sprintf("%d -> %d", g.h[dp.project], hsh)

if _, ex := rels[r]; !ex {
Expand Down Expand Up @@ -91,3 +91,24 @@ func (dp gvnode) label() string {

return strings.Join(label, "\n")
}

// Ensure that the literal string prefix is a path tree match and
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though this isn't exported, it should follow the Go standard of having the comment block begin with the func name.

// guard against possibilities like this:
//
// github.com/sdboyer/foo
// github.com/sdboyer/foobar/baz
//
// Verify that either the input is the same length as the match (in which
// case we know they're equal), or that the next character is a "/". (Import
// paths are defined to always use "/", not the OS-specific path separator.)
func isPathPrefixOrEqual(pre, path string) bool {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that I'm looking at this here, I realize the change in parameter order is terrible. Let's flip them so that it's the same as strings.HasPrefix() - string first, prefix second.

prflen, pathlen := len(pre), len(path)
if pathlen == prflen+1 {
// this can never be the case
return false
}

// we assume something else (a trie) has done equality check up to the point
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment should go away when strings.HasPrefix() moves into the func.

// of the prefix, so we just check len
return prflen == pathlen || strings.Index(path[prflen:], "/") == 0
}