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

animated cows #39

Open
bill-auger opened this issue Mar 21, 2023 · 8 comments
Open

animated cows #39

bill-auger opened this issue Mar 21, 2023 · 8 comments
Assignees
Labels
enhancement Make stuff better!
Milestone

Comments

@bill-auger
Copy link

bill-auger commented Mar 21, 2023

the future of the past is here today!

i have added an animation feature - would you be interested to adopt it?

demo:
https://asciinema.org/a/569000

@apjanke
Copy link
Collaborator

apjanke commented Mar 23, 2023

Oh, hmm. Hmm! That looks really cool. I'm interested in hearing more.

I'm of two minds about this: I'm conservative about adding new features to cowsay. Mostly out of respect for the (apparent) desires of cowsay's original Tony Monroe, who wanted cowsay to be "simple and silly", and kept the code pretty minimal. And for security & maintenance reasons - cowsay has an unknown amount of users, who want to use it casually, and this is a hobby project so I don't have time to thoroughly vet a lot of code. Plus to try to serve the cowsay user community - we don't have a mailing list or anything, so I have to just guess at what they'd want, and I err on the side of assuming they like it how it is and keeping it that way.

But this animated cow thing looks like a pretty cool feature, and I bet cowsay users would like it. Using animated cows would be neat, and this could be a cool new type of cow that would be fun for our users to to write definitions for; maybe even inspire a new little wave of cow-writing activity. And I dig your backflipping cow in particular here. So I'd definitely like to check it out, but no guarantee that I'll actually merge it.

Maybe we could ask on a couple forums or non-cowsay-specific mailing lists somewhere whether cowsay users would be interested in this. I really don't know where they hang out or how to find them.

Can you tell me a little about how your animated-cow feature works, in terms of implementation and dependencies? Is there code I can see now? I'm not very familiar with asciinema.

To be merged, any dependencies for this feature would probably have to be optional run-time dependencies. The only hard dependency that cowsay has now is (I think) Perl 5. (Plus asciidoc at authoring time, to build the man page, but users don't need that.) I'd like to keep it that way, but think it would be fine to optionally use other programs or Perl modules at run time to enable optional additional features like this.

@apjanke apjanke self-assigned this Mar 23, 2023
@apjanke apjanke added the enhancement Make stuff better! label Mar 23, 2023
@apjanke apjanke added this to Cowsay Mar 23, 2023
@bill-auger
Copy link
Author

i think people will enjoy playing with it - breathe a little new life into an old favorite toy

asciinema is unrelated - that was just a simple way to show a demo

i too am keen on keeping everything KISS, new and old - its not much new code and no new dependencies - it only imports the timer module from the PERL standard library

the changes are minimal, and the cow format is the same - people who know how to make cows now, will have no trouble making animated cows - as an after-thought, i even added a single-stepping feature to assist with making the animation frames (frames are just individual .cow files) - i documented it in the man page

i based it off the original code-base though - it may need changes; because it appears that you modified the cowsay script significantly, and you moved the cows/ directory - i will see if i can make the patches apply and will put it on github if it is not too much trouble - it is packaged in the parabola repos now - if you want to play with it now, grab this source-ball:
https://repo.parabola.nu/other/cowsay/cowsay-3.04-libre.tar.gz
https://repo.parabola.nu/other/cowsay/cowsay-3.04-libre.tar.gz.sig

('-libre'; because we delete the fan-art cows like beavis)

if youd like to see the patches, those are in this repo:
https://git.parabola.nu/abslibre.git/tree/libre/cowsay

i dont really know who to "reach out" to for promotion either - there are lots of popular forums and blogs that like this sort of "light news" (its-foss, hacker-news, phoronix, etc) - maybe one of them would like to blog about it - i showed it to people in a few IRC channels and they applauded it - that enough encouragement for me

who i would contact though, is distro devs - the ones i know about (arch and debian) are still using the abandoned upstream - i would suggest that they switch to your fork as their upstream - it appears to be the only active one, and the only one with a website; so they probably will - they probably just dont know about it

@apjanke
Copy link
Collaborator

apjanke commented Mar 27, 2023

I am enthused!

Your code changes here do indeed seem "minimal", no added-dependencies issues AFAICT, and I'm not seeing any obvious security concerns here. I read through the patches you linked, and unless I'm overlooking something major, this is looking like a win to me.

Just as a matter of precaution and conservatisim, I still won't merge a feature-add change like this without like a two-week waiting period where we can all just sit around and think about it.

i based it off the original code-base though - it may need changes; because it appears that you modified the cowsay script significantly

Uh, yeah. While I kept the "public" behavior of cowsay mostly the same – like, what CLI options the cowsay command supports and how it behaves in response to them; the "public API" of this tool – I made significant changes to the internal organization of the cowsay code. Some of it was to support the cowsay.d and COWPATH changes to make package-manager-based distribution work, and some of it was just to suit my personal taste in code.

If we decide to do this thing, and you've got patches against the original cowsay code, I'd be happy to translate those in to patches against my current cowsay code.

who i would contact though, is distro devs - the ones i know about (arch and debian) are still using the abandoned upstream - i would suggest that they switch to your fork as their upstream [...]

Yeah, totally. I'd guess that in 2023, like 98% of cowsay users currently get their cowsay from Linux distributions? Linux distros are totally what Cowsay should target if it wants wide distribution. But, I'm busy and tired these days, and I always saw this here fork of Cowsay as a "keep it going with an active-development fork that people can use if they want, and do some silly shit with Docker containers and whatnot" and not a "reach for the crown and become the real canonical" Cowsay". I don't think I currently have the energy to participate in an "advocate for Janke's cowsay fork to become the new official cowsay in Debian distributions" discussion.

@bill-auger
Copy link
Author

it is not so demanding - distros prefer an active upstream - they typically would switch from a dormant upstream to an active one, of their own volition, once they learn that an active one exists - that happened to me with freewheeling - no one asked distros to change to the new fork - they simply did so on their own - one email to each package maintainer may be convincing enough

all they would expect is that you would fix bugs, and ensure that it remains compatible with future PERL versions - cowsay probably does not have any bugs, unless your re-design introduced them - to keep it compatible with future PERL versions, that would be the most important factor - most legacy code-bases need only that - presumably that is the main reason you forked it, to keep it working

@apjanke
Copy link
Collaborator

apjanke commented Mar 29, 2023

Okay, you've got me interested here.

That sounds kinda like my experience. My own Ronn-NG fork of Ronn (another piece of abandonware I scooped up because I wanted it to keep running under current distros) got picked up as the new Ronn by Debian/Ubuntu a while back [1], and I didn't even know about it until like six months later when someone posted a GitHub Issue ticket asking me if I wanted to upstream Debian's patches.

And I'm already committed to keeping Cowsay compatible with any future Perl versions which are shipped as the default system Perl by major Linux distros or Mac Homebrew, so that seems like no additional burden.

presumably that is the main reason you forked it, to keep it working

More or less. That's half the story. It was: A) to keep it working with contemporary Linux distros and macOS Homebrew/MacPorts, B) to change its architecture a bit to work better with package managers and org/enterprise-managed systems in general, and C) to allow new cows to be committed! Moooo!

Anyway, I took a couple nights sleep to think on your "animated cows" idea, and given how lightweight your implementation here is, I am feeling very positive about it, and am leaning towards doing it, even if we can't do a user survey to establish community support.

...[bugs], unless your re-design introduced them...

Oh, it almost certainly did. The act of writing code is kinda the act of writing bugs. ;) But I'll be around to fix them.

A couple minor things:

  • -a - "a", as a very common letter, is a pretty "big" or "main" option to take for the single-stepping debugging-oriented option for animation. Could we pick a different more obscure letter like -S, or a --single-step style long option for that?
  • I think the namespace for animated cows and regular cows should be the same namespace. That is, there should be no static cow <foo>.cow file which has the same name as a <foo>/ animated cow directory. Looking at your code, I think that's your intention here, but IMHO we should make that explicit. And in the case of a conflict, as is possible with COWPATH, maybe we should be explicit how it's resolved: I say COWPATH order determines precedence first, and if for some weird reason a single COWPATH entry has both a static and animated cow, then the static cow takes precedence over the animated cow of the same name, and the animated cow effectively doesn't exist.
  • I think we should have an option, maybe -A/--no-animated, to disallow animated cows, to support users or contexts which can't usefully do animation. (Like piping cow content into a static email body or something.) In that case, I think attempts to use an animated cow should raise an error, list-cows features should exclude animated cows (but not expose static cows that were masked by animated cows), and the random-cow-selection features should ignore animated cows and only select from static cows.

So, I'm leaning toward, let's do this thing! Would you be interested in making a PR for it, or would you like me to hoover up the changes in to a PR myself with author credit for you in the git commit metadata, or...?

If we do this, I think your backflip cow would be a great initial example to include.


[1] Very possible this has been the high point of my open source hobby career so far. :)

@bill-auger
Copy link
Author

bill-auger commented Mar 29, 2023 via email

@apjanke
Copy link
Collaborator

apjanke commented Mar 31, 2023

... i would not raise an error or prevent any use-case, unless to allow it would actually break something ...

Yeah, the only use case I'm actually concerned about here is the "random cow selection" feature, where someone may be using that to get a cow (that they don't specify explicitly) for inclusion in a static-content context like an email body or web page or something, where they don't want multiple frames or ANSI terminal control characters. (Like, the "�[H�[2J�[3J" I see in your linked animation dump. Technically, I think those are not valid for inclusion in some text formats, and certainly won't be rendered "right" in some contexts.)

it was simply for tidiness, to avoid flooding the main directory

Yeah, I think we wanna keep $COWPATH clean and short.

to address that concern, i could even imagine an option to specify a frame by it's sequence number, like --frame=2

That sounds like a good approach to me, but also totally optional.

i dont mind doing [a PR] [...] it was an attempt to decide if i was going to host it myself, put it on github, or whatever

If you've got the time, go for the PR! I amenthusiastic about getting this merged in to this here cowsay project, and maybe get it in front of more users to play with.

my initial thought was for -l to report two lists: the original static cows list, and a second animated cows list - i was just being lazy to combine them - i dont see any reason to exclude them though, or to add a new --list-animated option

I think I see a use case for programmatically getting static-only cow lists, for where someone is programmatically integrating cowsay in to a context that only takes static cows and can't handle ANSI terminal control escape sequences, and they need to query which cows are valid for that context (used to be all cows, now it won't be). I'm happy to add the code for that myself though. For human-readable output, either way is probably fine, but I can see a use case for users being like "oooo, animated cows! which specific ones are there?" and wanting those to be called out separately.

i have described it as: "the latest advancement of ASCII cow technology"

And now we've got "moo-ving pictures" for your enhancement here...

@bill-auger
Copy link
Author

bill-auger commented Mar 31, 2023 via email

@apjanke apjanke added this to the The Future milestone Aug 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Make stuff better!
Projects
Status: No status
Development

No branches or pull requests

2 participants