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

Remove speech bubble from ascii #67

Open
oganm opened this issue Dec 18, 2018 · 17 comments
Open

Remove speech bubble from ascii #67

oganm opened this issue Dec 18, 2018 · 17 comments

Comments

@oganm
Copy link

oganm commented Dec 18, 2018

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
Copy link
Owner

sckott commented Dec 18, 2018

forces speech bubbles within the elements of the animals object

can you clarify? not sure what you mean

@oganm
Copy link
Author

oganm commented Dec 18, 2018

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
Copy link
Owner

sckott commented Dec 18, 2018

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
Copy link
Owner

sckott commented Feb 1, 2019

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

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

@aedobbyn
Copy link
Contributor

aedobbyn commented Feb 1, 2019

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
Copy link
Author

oganm commented Feb 1, 2019

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
Copy link
Author

oganm commented Feb 2, 2019

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

@aedobbyn
Copy link
Contributor

aedobbyn commented Feb 2, 2019

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
Copy link
Owner

sckott commented Feb 5, 2019

makes sense @aedobbyn

@aedobbyn
Copy link
Contributor

aedobbyn commented Feb 5, 2019

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
Copy link
Author

oganm commented Feb 6, 2019

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
Copy link
Contributor

aedobbyn commented Feb 6, 2019

Works for me, thanks @oganm

@aedobbyn
Copy link
Contributor

aedobbyn commented Mar 5, 2019

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
Copy link
Owner

sckott commented Mar 5, 2019

sounds good @aedobbyn

@oganm
Copy link
Author

oganm commented Mar 5, 2019

yep. it's fine 👍

@aedobbyn
Copy link
Contributor

aedobbyn commented Mar 6, 2019

Awesome, thanks guys!

@sckott
Copy link
Owner

sckott commented Oct 14, 2023

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

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

3 participants