Skip to content

Makefile colorization: fix old issues and improvement (~4 changes) #33327

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.

Sign up for GitHub

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

Jump to bottom
Closed
fadeevab wants to merge 8 commits into from
Closed

Makefile colorization: fix old issues and improvement (~4 changes) #33327

fadeevab wants to merge 8 commits into from

Conversation

@fadeevab
Copy link
Contributor

@fadeevab fadeevab commented Aug 29, 2017 •
edited
Loading

  1. This pull request fixes the following issue: Makefile parsing bug? #25469
  2. Additional improvements according to Makefile documentation.

Before:
before
After:
after

Total list of changes at the moment:

  1. Allow spaces before ifeq, define, endif, ended, else.
  2. Handle braces properly inside the variable, e.g. $(info ()()(())).
  3. Handle backslash inside end-of-line inside the $(variable).
  4. Handle $$ and special variables according to Makefile documentation.

@msftclas
Copy link

msftclas commented Aug 29, 2017

@fadeevab,
Thanks for your contribution.
To ensure that the project team has proper rights to use your work, please complete the Contribution License Agreement at https://cla.microsoft.com.

It will cover your contributions to all Microsoft-managed open source projects.
Thanks,
Microsoft Pull Request Bot

@fadeevab
Copy link
Contributor Author

fadeevab commented Aug 29, 2017

@aeschli Pull request to textmate is updated as well (textmate/make.tmbundle#15)

@aeschli
Copy link
Contributor

aeschli commented Aug 29, 2017

Thanks @fadeevab . I will update the grammar next week (we're currently in the end game week).

@aeschli aeschli added this to the September 2017 milestone Aug 29, 2017
@aeschli
Copy link
Contributor

aeschli commented Aug 29, 2017

BTW, no need to make a PR. It's also good enough if you file me an issue and ask to to run the update script

@msftclas
Copy link

msftclas commented Aug 29, 2017

@fadeevab, thanks for signing the contribution license agreement. We will now validate the agreement and then the pull request.

Thanks, Microsoft Pull Request Bot

FadeevAlexandr and others added 4 commits August 31, 2017 00:49
@FadeevAlexandr
Handle back-slash before end-of-line inside the $(variable).
6ce30e6
Handle "$$" and special variables.
cef201b 
Makefile use the following rule to parse variables:

 * A dollar sign followed by a character other than a dollar sign, open-parenthesis or open-brace treats that single character as the variable name.

$$, $@, $v, $*: highlight as a single variable.
$(): highlight as interpolation and parse content inside the braces.
$$(): "$$" is highlighted to show it's escaped, but parse content inside braces for convenient representation.
New test-cases: handle $$.
da609f4
Makefile test expected result is updated.
1d181f1
@fadeevab
Copy link
Contributor Author

fadeevab commented Aug 31, 2017

I added commits and test cases. But I didn't ported them to TextMate yet. Wait until I update TextMate. The reason is that I develop in VSCode firstly. It's unreal to develop in XML format initially.

@fadeevab
Copy link
Contributor Author

fadeevab commented Aug 31, 2017 •
edited
Loading

@aeschli , FYI, TextMate xml is updated, you can check here the test

@fadeevab fadeevab changed the title Makefile colorization (fix old issues): 1. handle braces inside variable, 2. spaces before ifdef Makefile colorization: fix old issues and improvement (~4 changes) Sep 14, 2017
@fadeevab
Copy link
Contributor Author

fadeevab commented Sep 14, 2017

@aeschli You can check it now.

Total list of changes at the moment:

  1. Allow spaces before ifeq, define, endif, ended, else.
  2. Handle braces properly inside the variable, e.g. $(info ()()(())).
  3. Handle backslash inside end-of-line inside the $(variable).
  4. Handle $$ and special variables according to Makefile documentation.

I didn't updated the latest expected result according to the new test, thus the latest build failed.

@aeschli
Copy link
Contributor

aeschli commented Sep 18, 2017

Closing the PR. We are in sync with https://github.com/fadeevab/make.tmbundle

@aeschli aeschli closed this Sep 18, 2017
@fadeevab
Copy link
Contributor Author

fadeevab commented Sep 18, 2017

Thank you!

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

Reviewers

No reviews

Assignees

@aeschli aeschli

Labels

None yet

Projects

None yet

Milestone

September 2017

Development

Successfully merging this pull request may close these issues.

4 participants

@fadeevab @msftclas @aeschli @FadeevAlexandr