box-cli-maker icon indicating copy to clipboard operation
box-cli-maker copied to clipboard

failing to get `term` width for wrapping shouldn't be fatal

Open avivklas opened this issue 2 years ago • 2 comments

hello! I encountered an issue using this lib, looked for the source of it, and found out something that might be a wrong decision.

here's what happened to me:

  • I tried to print a box with this code:
box.New(box.Config{
	Py:            0,
	Px:            5,
	ContentAlign:  "Center",
	Type:          "Double",
	TitlePos:      "Inside",
	AllowWrapping: true,
}).Println(title, msg)
  • I started my app in a container started by docker-compose.
  • my app fataled with the message inappropriate ioctl for device
  • I looked in the code and found out that if I'm setting AllowWrapping to true and an error is returned from term.GetSize,the code calls to log.Fatal:
	// Allow Wrapping according to the user
	if b.AllowWrapping {
		// If limit not provided then use 2*TermWidth/3 as limit else
		// use the one provided
		if b.WrappingLimit != 0 {
			lines = wrap.String(lines, b.WrappingLimit)
		} else {
			width, _, err := term.GetSize(int(os.Stdout.Fd()))
			if err != nil {
				log.Fatal(err)
			}
			lines = wrap.String(lines, 2*width/3)
		}
	}

IMO, a library of such kind should not panic but return an error instead. let the user decide what to do.

avivklas avatar Aug 30 '23 17:08 avivklas

You are correct that this should be handled by the user, not by the library. I made this library 2-3 years ago without much knowledge of Go. I have been planning to do this in v3 because it is a big change, and it needs to be done with a lot of consciousness. Here is the issue tracker for this: https://github.com/Delta456/box-cli-maker/issues/26

Delta456 avatar Aug 30 '23 18:08 Delta456