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

Improve argument parsing #1

Merged
merged 2 commits into from
Sep 11, 2020
Merged

Improve argument parsing #1

merged 2 commits into from
Sep 11, 2020

Conversation

nonzerofloat
Copy link
Contributor

@nonzerofloat nonzerofloat commented Sep 9, 2020

crumbs idea.txt is trying to read from stdin, not file idea.txt. I changed to read file if file is specified, or to read from standard input if not specified.

@lucasepe
Copy link
Owner

lucasepe commented Sep 10, 2020

Hi! @gopherclass thanks for your PR.

Could you please explain to me what is the behavior that doesn't suit you?

I tried your PR but if I type crumbs and then enter, it prints an error message to the console error: operation performed on character device (at least on Linux, I'm on Debian) and this is an unwanted behavior.

Crumbs as is

Without args

shows the help - 👌 expected behavior

$ crumbs

...shows the help...

With an arg (case: a file that does not exists)

shows the error message no such file or directory - 👌 expected behavior

$ crumbs file_that_does_not_exists.txt

error: open file_that_does_not_exists.txt: no such file or directory

With an arg (case a file that exists)

convert the crumbs text file to a graphviz dot script - 👌 expected behavior

$ crumbs file_that_exists.txt
graph  {
	......
        ......
}

Crumbs with your PR

Without args

shows an error - 👾 unwanted behavior

$ crumbs
error: operation performed on character device

@nonzerofloat
Copy link
Contributor Author

nonzerofloat commented Sep 10, 2020

I think it is better not to check if stdin is a character device.

I'm on Windows with msys environment. I made chardev.go that shows mode of stdin to investigate what causes the problem. On mintty (default terminal emulator in msys environment), stdin is connected via pipe and is not reported to be a character device. On winpty (other terminal emulator), stdin is shown to be a character device. I tested only on mintty. I didn't know the subtle difference.

//+build ignore

// chardev.go
package main

import (
	"os"
	"fmt"
)

func assert(err error) { if err != nil { panic(err) } }

func main() {
	fi, err := os.Stdin.Stat()
	assert(err)
	fmt.Println(fi.Mode())
	fmt.Println(fi.Mode() & os.ModeCharDevice > 0)
}
$ go build -o chardev chardev.go
$ ./chardev
prw-rw-rw-
false
$ winpty ./chardev
Dcrw-rw-rw-
true
// Powershell and traditional windows cmd outputs the same result with winpty ./chardev

I removes mode-checking code.

In the case without args, an attempt is made to read from standard input. Help message is not displayed.

@lucasepe
Copy link
Owner

Ok now it's clear. Great 👍

@lucasepe lucasepe merged commit 1e318e7 into lucasepe:master Sep 11, 2020
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

Successfully merging this pull request may close these issues.

2 participants