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

Make make help colorful and simple #901

Closed
wants to merge 4 commits into from

Conversation

szepeviktor
Copy link
Contributor

Colorful and simple.

Makefile Outdated
@echo "\033[33mUsage:\033[0m\n make TARGET\n\n\033[32m#\n# Commands\n#---------------------------------------------------------------------------\033[0m\n"
@fgrep -h "##" $(MAKEFILE_LIST) | fgrep -v fgrep | sed -e 's/\\$$//' | sed -e 's/##//' | awk 'BEGIN {FS = ":"}; {printf "\033[33m%s:\033[0m%s\n", $$1, $$2}'
@echo -e "\033[33mUsage:\033[0m\n make TARGET\n\n\033[32m#\n# Commands\n#---------------------------------------------------------------------------\033[0m\n"
@sed -n -e 's|^\(.\+:\) ##\(.\+\)$$|\o033[33m\1 \o033[0m\2|p' $(MAKEFILE_LIST)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One sed does it all.

@szepeviktor
Copy link
Contributor Author

szepeviktor commented Nov 5, 2023

Why does that fail?
On my machine

$ make help
\033[33mUsage:\033[0m\n  make TARGET\n\n\033[32m#\n# Commands\n#---------------------------------------------------------------------------\033[0m\n

@theofidry
Copy link
Member

The result after running make help before/after the changes of your PR for me:

Screenshot 2023-11-05 at 22 42 55

Screenshot 2023-11-05 at 22 43 04

@szepeviktor
Copy link
Contributor Author

Is your shell Bash?

@theofidry
Copy link
Member

zsh*

@szepeviktor
Copy link
Contributor Author

I see.
echo is not portable. https://unix.stackexchange.com/a/700679

@theofidry
Copy link
Member

theofidry commented Nov 5, 2023

I indeed use this form in another project https://github.com/theofidry/makefile/blob/main/Makefile#L42 I probably was just too lazy to update it everywhere and forgot about it.

Note I also didn't use sed, because it behaves different on OSX which is super annoying. And actually I think it's the culprit for the failure. The CI works fine as it's ubuntu, but locally I get the same as the "after" screenshot from before.

@szepeviktor
Copy link
Contributor Author

Note I also didn't use sed, because it behaves different on OSX

Yes. GNU sed has an option: sed --posix to disable GNU extensions.
Indeed it does not work with that option!

@theofidry
Copy link
Member

TIL

@szepeviktor
Copy link
Contributor Author

The "one or more" + sign is the culprit.

I give up. Sorry.

@szepeviktor szepeviktor closed this Nov 5, 2023
@theofidry
Copy link
Member

So I think if you revert just the @sed change it's still a mergeable change :) I would be happy to simplify the current grep command but I admit I also didn't care enough to try to figure a way to do so (it really didn't look simple and the sort of issues I could spend hours on)

@szepeviktor szepeviktor deleted the patch-1 branch November 5, 2023 23:29
@theofidry
Copy link
Member

I give up. Sorry.

No worries, it's a nasty one

@szepeviktor
Copy link
Contributor Author

it really didn't look simple

Compatibility is a hot topic!
I keep investigating in my secret kitchen ...

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