andrewpillar/cl

cl does not parse tabs

wkhere opened this issue · 5 comments

wkhere commented

Hi!
That's a neat tool.

However, would be cool if it could parse tabs, not only spaces, in the ClFile. Currently it doesn't:

dial tcp: lookup XX.YY.TLD        ~/.ssh/id_ed25519: no such host
andrewpillar commented

Thanks for opening up this issue, just pushed a commit to support this, the tests I added seem to pass with tabs, spaces, and a mix of both. Also, commenting is now a thing in the ClFile, simply prefix a line with #.

Commits

  • ❤️1
wkhere commented

Hey, that's cool, thank you!

Btw, in this last commit I noticed "user" became lowercase in the os.Getenv call, not sure if that wasn't a typo or something.

andrewpillar commented

Noticed that, already fixed in the most recent commit here.

  • 👍1
wkhere commented

Hey, I believe the bug is still there. I wasn't checking it before against my example ClFile, which has tabs between hostname and the identity path - like at the top of this thread.

Looking at the test, you were checking for the tabs at the start of the line, but not between the fields.

Now I get:

cl.out: dial tcp: lookup XX.YY.TLD      ~/.ssh/id_rsa: no such host

for just 1 tab between host and identity, and:

cl.out: dial tcp: lookup XX.YY.TLD       : no such host

for tab+space.. So there is definitely a different buggy behavior for each of the cases I just described.

I think this parsing is a bit complicated. With such a simple format, why not to use strings.Fields and have all the fields separated regardless of the amount of whitespace between them:

func Fields(s string) []string
    Fields splits the string s around each instance of one or more consecutive
    white space characters, as defined by unicode.IsSpace, returning a slice of
    substrings of s or an empty slice if s contains only white space.

My another 3 cents (or pennies) would be, it's good to separate lexing and parsing. Lexing in this case is just splitting the fields and seeing the colon. Parsing would be treating the line with a colon as a heading, and other line as the host+identity definition. It's harder to reason about the code if such logic is intermixed with lexing..

Hope this helps..

andrewpillar commented

The reason I didn't use strings.Fields is because I initially used spaces for my own ClFile. Simply an overhang from the initial implementation, didn't expect people to use a mix of tabs and spaces. Pushed up a new commit now, so should work.

  • 👍1