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

Newcows #40

Merged
merged 21 commits into from
Dec 20, 2018
Merged

Newcows #40

merged 21 commits into from
Dec 20, 2018

Conversation

jasonkhanlar
Copy link
Contributor

Per my comment at #38 (comment)
And after my previous pull request minutes ago #39
here are a bunch of additional *.cow files pulled from the other repos listed.

@piuccio
Copy link
Owner

piuccio commented Oct 26, 2018

wow, I don't usually accept new cows but this is such an amazing PR. Would you mind just adding these new files on browsers.js. There's a list there that is needed for people using cowsay in the browser

…/articles/1704/15/news028.html and I see it is ascii art for a picture illustrating pouring of the milk container.
@jasonkhanlar
Copy link
Contributor Author

for i in *.cow;do
    ucname=$(echo $i|sed "s|[-.]|_|g"|sed "s|^\(.*\)_cow$|\1|"|tr a-z A-Z);
    echo "export { default as $ucname } from '.cows/$i';";
done

@piuccio
Copy link
Owner

piuccio commented Nov 12, 2018

can you please remove chop($eyes) from the files and the EOC at the bottom of new cows? After that it's ready to be merged

@jasonkhanlar
Copy link
Contributor Author

can you please remove ..... the EOC at the bottom of new cows?

I am not sure if that is a good idea. I notice all the existing cowfiles have EOC line at the end

Also lib/replacer.js:

function extractTheCow (cow) {
	cow = cow.replace(/\r\n?|[\n\u2028\u2029]/g, "\n").replace(/^\uFEFF/, '');
	var match = /\$the_cow\s*=\s*<<"*EOC"*;*\n([\s\S]+)\nEOC\n/.exec(cow);

	if (!match) {
		console.error("Cannot parse cow file\n", cow);
		return cow;
	} else {
		return match[1].replace(/\\{2}/g, "\\").replace(/\\@/g, "@").replace(/\\\$/g, "$");
	}
}

The match variable will evaluate to false without the EOC line at end?

@piuccio
Copy link
Owner

piuccio commented Nov 18, 2018

Oh I forgot the EOC was part of the regexp, I'll review your changes tomorrow

@jasonkhanlar
Copy link
Contributor Author

bump

@piuccio
Copy link
Owner

piuccio commented Dec 17, 2018

oh totally forgot, I'll have a look soon

@piuccio
Copy link
Owner

piuccio commented Dec 18, 2018

you know what I'm thinking? Maybe we should allow cows from node modules, kinda like plugins.
You'll be able to create your own repo, change it as you like and cowsay will use your cows as well as the default ones.

What do you think? I can do some changes in the code to allow that

@piuccio piuccio merged commit 3eba619 into piuccio:master Dec 20, 2018
ofarukcaki pushed a commit to ofarukcaki/cowsay that referenced this pull request Jul 29, 2023
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

Successfully merging this pull request may close these issues.

2 participants