cowsay-org / cowsay

apjanke's fork of the classic cowsay project
http://cowsay.diamonds
GNU General Public License v3.0
72 stars 15 forks source link

animated cows #39

Open bill-auger opened 1 year ago

bill-auger commented 1 year ago

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 commented 1 year ago

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.

bill-auger commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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:

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 commented 1 year ago

Could we pick a different more obscure letter like -S, or a --single-step style long option for that?

np - im not married to the letter 'a' - i just chose an unused letter, vaguely "mnemonical"

  • 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.

putting animated cows in a separate directory, was not about name-spacing, in the formal sense - it is of no consequence - it was simply for tidiness, to avoid flooding the main directory - an animation may consist of dozens of .cow files - the backflip cow has over 20 "frames" - just a few of those in the main directory, would make ls $COWPATH unwieldy to read

name ambiguity as you described, could be avoided by requiring the directory name to end with .cow - that would make name clashes impossible

  • 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

surely the design could use some refinement - i dont think that specific use-case would be problematic though

if you pipe an animated cow, you will simply get all the frames

i would not raise an error or prevent any use-case, unless to allow it would actually break something - this is the result of piping an animated cow - its not pretty; but its not broken either https://termbin.com/1tcf

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

$ cowsay -f backflip-cow --frame=2 moo

that would prevent animations from playing entirely, while allowing animated cows to be piped cleanly (outputting only a single frame #2, the same output of a normal cow file)

list-cows features should exclude animated cows

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

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...?

i dont mind doing it - i forked the cowsay-org code-base on github yesterday - i asked first, mainly because i had not published anywhere yet - it was an attempt to decide if i was going to host it myself, put it on github, or whatever

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

well yea, generally you should always include an example, if only for documentation or tutorial purposes - there only happens to be that one example now

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

i have described it as: "the latest advancement of ASCII cow technology" and: "next-generation ASCII cow technology"

apjanke commented 1 year ago

... 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 "" 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 commented 1 year ago

On Thu, 30 Mar 2023 21:25:16 -0700 Andrew wrote:

... 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

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.

that could be the way to make the "random cow" feature robust, even with the animated cows - make the "random cow" feature always render frame #1 only