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

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

Merged
merged 1 commit into from
Feb 4, 2019

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.

None yet

3 participants