Skip to content

Makefile grammar updating: tests are updated about the handling @, - and +#65629

Merged
alexr00 merged 1 commit intomicrosoft:masterfrom
fadeevab:makefile-update
Feb 4, 2019
Merged

Makefile grammar updating: tests are updated about the handling @, - and +#65629
alexr00 merged 1 commit intomicrosoft:masterfrom
fadeevab:makefile-update

Conversation

@fadeevab
Copy link
Contributor

@fadeevab fadeevab commented Dec 23, 2018

  1. @, - and + in the beginning of recipes are colored.
  2. Shell in recipes is not colored by shellscript extension anymore:
    improper colorizing of Makefile variables, low suitability.

About the pull request: the diff is mostly about the disabling a shellscript colorizing inside the Makefile, so there are joined lines like a "c": "(sed -nre 's/some regex with (group)/\1/p')". You can see echo "#" and '#'... without shellscript colorization, but it's expected: you can see the last line on the screenshot looks better with proper highlight of Makefile variables, which is desirable behavior.

Tests will work after updating the grammar from the upstream.

Shellscript coloring makes more harm than advantage: Makefile treats $(variable) with absolute high priority under processing, and shellscript extension is not able to handle those rules, confusing the developer. Also there are problems to consistently colorize $(shell ..) and !=.

Before:
before
After:
after

@fadeevab
Copy link
Contributor Author

fadeevab commented Dec 23, 2018

@alexr00, @aeschli

@darkyuhoo01
Copy link

@alexr00

@alexr00 alexr00 self-requested a review January 3, 2019 14:43
@alexr00
Copy link
Member

alexr00 commented Jan 4, 2019

I've updated the grammar. I checked the rebase and it doesn't seem too bad. You should be able to rebase and push to resolve the conflicts.

@fadeevab
Copy link
Contributor Author

fadeevab commented Jan 4, 2019

Ok, thank you, give me a time

… - and +.

1. @, - and + in the beginning of recipes are colored.
2. Shell in recipes is not colored by shellscript extension anymore:
improper colorizing of Makefile variables, low suitability.
@fadeevab
Copy link
Contributor Author

fadeevab commented Feb 3, 2019

@alexr00, Okay, eventually, makefile test is rebased, PR should be merged without conflicts.

@alexr00 alexr00 added this to the February 2019 milestone Feb 4, 2019
@alexr00 alexr00 merged commit f3552ec into microsoft:master Feb 4, 2019
@alexr00
Copy link
Member

alexr00 commented Feb 4, 2019

Thank you!

@github-actions github-actions bot locked and limited conversation to collaborators Mar 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants