sckott / cowsay

cowsay w/ more animals, in R
https://sckott.github.io/cowsay/
Other
301 stars 48 forks source link

Remove speech bubble from ascii #67

Open oganm opened 5 years ago

oganm commented 5 years ago

It seems like the default way of specifying animals forces speech bubbles within the elements of the animals object, causing the output to be less pretty compared to the command line alternative.

< hello >
 -------
        \   ^__^
         \  (oo)\_______
            (__)\       )\/\
                ||----w |
                ||     ||

vs

 ----- 
hello 
 ------ 
    \   ^__^ 
     \  (oo)\ ________ 
        (__)\         )\ /\ 
             ||------w|
             ||      ||

This can be solved by giving the responsibility of creating the speech bubble to say function. Would this be a welcome change? I can look into it if I find some spare time.

sckott commented 5 years ago

forces speech bubbles within the elements of the animals object

can you clarify? not sure what you mean

oganm commented 5 years ago

You define an animal as

---
%s
---
 \
  \
   animal

Which means the speech bubble will look the same no matter what the input text is. For instance

---
extremely long input that this bubble is clearly not designed for
---
 \
  \
   animal

While if instead animals were defined as

%s
   \
    \
     animal

You could replace the entire speech bubble with one that looks appropriate to the input size.

 _________________________________________________________________________
< extremely long input in a bubble created based on input character count >
 -------------------------------------------------------------------------
 \
  \
   animal
sckott commented 5 years ago

Okay.

My cli cowsay has vertical bars instead of carrots, not important really, but just to show you

lorem -s 5 | cowsay
 _________________________________________
/ Lorem ipsum dolor sit amet,             \
| consectetuer adipiscing elit! Aenean    |
| commodo ligula eget dolor? Aenean       |
| massa; Cum sociis natoque penatibus et  |
| magnis dis parturient montes, nascetur  |
| ridiculus mus? Donec quam felis,        |
| ultricies nec, pellentesque eu, pretium |
\ quis, sem!                              /
 -----------------------------------------
        \   ^__^
         \  (oo)\_______
            (__)\       )\/\
                ||----w |
                ||     ||

I think making this the default would be good. it'd be good to have the bubble envelop the entire string

wrt letting the user specify, can you give a use case? and might it be difficult for the user to know how to give the bubble? i guess you could present options for the bubble that are a set that we know work well.

sckott commented 5 years ago

any thoughts on this @oganm ? do you want to submit a PR?

opinions @aedobbyn ? do you want this change or no?

aedobbyn commented 5 years ago

Interesting idea! I think I like it.

@sckott's cli cowsay I think looks good because it's more compact than the full width of the terminal.

As far as implementing something like that, for the top and bottom of the bubble I guess you'd just make a line measured by something like

min(options("width")$width, some_max_width)

to ensure that some_max_width isn't greater than the user's console width, where either we or the user determine some_max_width.

Then add |s for all lines except the first and last, which get / and \.

As far as splitting the text up into lines I'm not totally sure but I don't think you wouldn't want to split up the text exactly evenly because that would spread words across multiple lines. So you'd probably want to go with splitting on the last space character without going over min(options("width")$width, some_max_width). Does that sound about like what you had in mind @oganm?

oganm commented 5 years ago

Not sure how much it should be based on the terminal size as it might get too unwieldy. Something adjustable with a smaller default would probably look better. The command cowsay seems to limit it to 44 characters by default

oganm commented 5 years ago

also sorry about a lack of a PR. didn't have time to look into it

aedobbyn commented 5 years ago

Yeah sorry I didn't explain well. The idea is that the maximum width would be some_max_width or 44. We'd have to shrink the speech bubble to less than 44 if the user's console is smaller than that, though. If their console is smaller, then the speech bubble width would just be the user's console width. Does that make sense?

sckott commented 5 years ago

makes sense @aedobbyn

aedobbyn commented 5 years ago

Cool cool. If you want to put together a PR @oganm, go for it. Another idea would be to kick it to participants at an upcoming unconf I'm planning on going to since this would be a cool thing for people to talk through implementing. (The idea with this unconf is to let participants contribute to existing packages.) It's your idea, though, so very much up to you!

oganm commented 5 years ago

If I'm not done by that time feel free to hand it to the unconf. Will try to take a look before that

aedobbyn commented 5 years ago

Works for me, thanks @oganm

aedobbyn commented 5 years ago

Hey @oganm and @sckott -- planning on using this issue as a jumping-off point at the upcoming unconf I mentioned, if that's okay with you both. Happy to keep you guys in the loop next weekend as people work on it, if you want!

https://github.com/chirunconf/chirunconf19/issues

sckott commented 5 years ago

sounds good @aedobbyn

oganm commented 5 years ago

yep. it's fine :+1:

aedobbyn commented 5 years ago

Awesome, thanks guys!

sckott commented 11 months ago

@aedobbyn It would be nice to make this change. Would you want to do it, and have time to?