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

scripts: fix echo and add executable flag #114

Merged
merged 1 commit into from
Dec 13, 2021

Conversation

marzzzello
Copy link
Contributor

@marzzzello marzzzello commented Dec 10, 2021

The echo strings include \n which is only interpreted with the -e option

@@ -2,12 +2,10 @@
# file: lock.sh
# description: Lock dependencies and export requirements.

echo "# THE FILE WAS GENERATED BY POETRY, DO NOT EDIT!\n\n" > src/requirements.txt
echo "# THE FILE WAS GENERATED BY POETRY, DO NOT EDIT!\n\n" > src/requirements-dev.txt
echo -e "# THE FILE WAS GENERATED BY POETRY, DO NOT EDIT!\n\n" > src/requirements.txt
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pardon my ignorance, but what does this do? And why is it needed?

I'm happy to merge it if you think it'll improve the script.

@marzzzello
Copy link
Contributor Author

It will fix the interpretation of the newlines ("...\n\n")
Without this the \n are not convertet into newlines. At least on Arch Linux.

@VikashKothary
Copy link
Member

VikashKothary commented Dec 13, 2021

That's good to know. I was doing some reading and apparently echo isn't very consistent between versions which is why it works on Ubuntu for me but not on Arch Linux. -e seems to work on both which is good.

In the future, we can maybe switch to printf which is meant to be more consistent between versions.

Thanks for your contribution.

@VikashKothary VikashKothary merged commit 0753292 into ankicommunity:develop Dec 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants