Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

strange tree #2

Closed
mattn opened this issue Aug 25, 2017 · 12 comments
Closed

strange tree #2

mattn opened this issue Aug 25, 2017 · 12 comments

Comments

@mattn
Copy link

mattn commented Aug 25, 2017

This is output from command lile https://github.com/lileio/lile

$ find foo
foo/.gitignore
foo/.travis.yml
foo/Dockerfile
foo/Makefile
foo/foo/cmd/root.go
foo/foo/cmd/serve.go
foo/foo/cmd/subscribe.go
foo/foo/cmd/up.go
foo/foo/main.go
foo/foo.proto
foo/server/server.go
foo/server/server_test.go
foo/subscribers/subscribers.go
.
├── server
│   ├── server.go
│   └── server_test.go
├── subscribers
│   └── subscribers.go
├── foo
│   ├── cmd
│       ├── root.go
│       ├── serve.go
│       ├── subscribe.go
│       └── up.go
│   └── main.go
├── foo.proto
├── Makefile
├── Dockerfile
├── .travis.yml
└── .gitignore

maybe this is expected?

.
├── server
│   ├── server.go
│   └── server_test.go
├── subscribers
│   └── subscribers.go
├── foo
│   ├── cmd
│   │   ├── root.go
│   │   ├── serve.go
│   │   ├── subscribe.go
│   │   └── up.go
│   └── main.go
├── foo.proto
├── Makefile
├── Dockerfile
├── .travis.yml
└── .gitignore
@xlab xlab closed this as completed in 2075aa9 Aug 25, 2017
@xlab
Copy link
Owner

xlab commented Aug 25, 2017

Good catch, thanks!

@mattn
Copy link
Author

mattn commented Aug 25, 2017

Thanks for fixing this. BTW, why you use 0x00 0xa0 for indentation?

fmt.Fprintf(wr, "%s   ", EdgeTypeLink)

On some character-set, for example japanese, it is shown as ?.

@xlab
Copy link
Owner

xlab commented Aug 25, 2017

It's expected to be Unicode friendly. As far as I can remember I use the same tree util uses.
Any ideas?

@mattn
Copy link
Author

mattn commented Aug 25, 2017

This is output on Windows. Do you want workaround fixes only on Windows?

@mattn
Copy link
Author

mattn commented Aug 25, 2017

One more thing. Corner string is two cells on some locales.

@xlab
Copy link
Owner

xlab commented Aug 25, 2017

This package will never be platform-specific, as its output is statically defined and may be parsed in logs during process mining.

The data it outputs is prevailing over the looks.

@mattn
Copy link
Author

mattn commented Aug 25, 2017

So you don't want fixing for east asian ambiguous widths. Right?

I don't have strong opinion about this. Just confirming how do you think. :)

@xlab
Copy link
Owner

xlab commented Aug 25, 2017

I'm ready to fix any issue as long as it's universal for all possible platforms and locales. If we can replace the spacing char so it will work fine with Unicode and asian encodings then let's do it.

My point is that there should be no exception cases.

@mattn
Copy link
Author

mattn commented Aug 25, 2017

Most easy way to fix this issue is:

diff --git a/treeprint.go b/treeprint.go
index 467c87d..7412518 100644
--- a/treeprint.go
+++ b/treeprint.go
@@ -6,6 +6,9 @@ import (
 	"fmt"
 	"io"
 	"reflect"
+	"runtime"
+
+	"github.com/mattn/go-runewidth"
 )
 
 type Value interface{}
@@ -156,12 +159,25 @@ func printNodes(wr io.Writer,
 func printValues(wr io.Writer,
 	level int, levelsEnded []int, edge EdgeType, meta MetaValue, val Value) {
 
+	var edgeTypeLink, spaces string
+	if !runewidth.IsEastAsian() {
+		spaces = "   "
+		if runtime.GOOS == "windows" {
+			edgeTypeLink = string(EdgeTypeLink) + "   "
+		} else {
+			edgeTypeLink = string(EdgeTypeLink) + "   "
+		}
+	} else {
+		spaces = "       "
+		edgeTypeLink = string(EdgeTypeLink) + "     "
+	}
+
 	for i := 0; i < level; i++ {
 		if isEnded(levelsEnded, i) {
-			fmt.Fprint(wr, "    ")
+			fmt.Fprint(wr, spaces)
 			continue
 		}
-		fmt.Fprintf(wr, "%s   ", EdgeTypeLink)
+		fmt.Fprint(wr, edgeTypeLink)
 	}
 	if meta != nil {
 		fmt.Fprintf(wr, "%s [%v]  %v\n", edge, meta, val)

@mattn
Copy link
Author

mattn commented Aug 25, 2017

One another fix is using ASCII letters. For example + for , | for .

@xlab
Copy link
Owner

xlab commented Aug 25, 2017

Okay, as the output of this utility is predictable, you could hand off both transformations to the application logic which can be OS-specific.

tree := treeprint.New()
// do stuff
log.Println(applyFilter(tree.String()))

func applyFilter(str string) string {
    // uses github.com/mattn/go-runewidth
    // checks runtime.GOOS
    // replaces treeprint.EdgeTypeLink and other with mapped ASCII chars, etc
}

I will add link to this issue in README. Thanks for the solution.

@mattn
Copy link
Author

mattn commented Aug 25, 2017

I don't mind about fixing my apps. However, most of packages using treeprint will have to include this workaround.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants