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

Support whitespace before target name for make completions #858

Merged
merged 2 commits into from
May 30, 2024

Conversation

ddanier
Copy link
Contributor

@ddanier ddanier commented May 30, 2024

TL;DR: The "simple" example from https://www.gnu.org/software/make/manual/html_node/Simple-Makefile.html is currently not compatible with the custom completion script found in custom-completions/make/make-completions.nu. This PR tries to fix that.

As I was working on nur (https://github.com/ddanier/nur) and the nurify script to convert to nur from different task runners (https://github.com/ddanier/nur/blob/main/scripts/nurify.nu) I wanted to create a good way to convert from using make. So I thought the make completion would for sure implement a good way to get a list of all possible make targets. Hence I started looking at custom-completions/make/make-completions.nu.

Then I searched for a good documentation for how Makefiles work, as the last time I was using this myself is about 5 to 10 years ago. If you for example look at the documentation on gnu.org you may find examples of Makefiles not working with the current autocompletion. See https://www.gnu.org/software/make/manual/html_node/Simple-Makefile.html for example, the "simple" example they provide.

The reason for this not working is that the targets use some whitespace after the target name. This is somehow allowed and thus valid. See https://www.gnu.org/software/make/manual/html_node/Rule-Introduction.html for a quick overview about how the Makefiles syntax works. I quickly checked this to ensure make actually parses this correctly, it really does.

This means that the current make completion does miss support for the "simple" example provided my make itself. So I went on to fix this.

My suggested solution is:

  • Filter all lines by regex '^[\w\.-]+\s*:' to ensure possible targets
    • start with some word (also allowing . and -)
    • may have some whitespaces after the word
    • has ":" after this
  • Split by the ":"
  • Use first column
  • Trim the remaining target name to remove those nasty whitespaces
  • Use result for completion

For me this did fix the issue with the "simple" Makefile, allowing me to put this into my nurify script.

Would be nice to get this "backported" to nu scripts as well. Might help others 😉

@fdncred
Copy link
Collaborator

fdncred commented May 30, 2024

Looks good. Thanks! I'm excited about nur and it's fun to see people using nushell in creative ways.

@fdncred fdncred merged commit e4dbec6 into nushell:main May 30, 2024
1 check failed
@ddanier ddanier deleted the dev/better-makefile-parsing branch May 30, 2024 19:45
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.

None yet

2 participants