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

Commas in macros not separating parameters properly #324

Closed
tristan957 opened this issue Jul 23, 2019 · 14 comments · Fixed by #325
Closed

Commas in macros not separating parameters properly #324

tristan957 opened this issue Jul 23, 2019 · 14 comments · Fixed by #325
Labels
Syntax: C For bugs that only occur in C Syntax: C++ issue exists for C++

Comments

@tristan957
Copy link

I am noticing weird behavior on the last comma in the macro definition, or maybe the previous two commas are wrong. Is TRUE supposed to be recognized as a boolean value in this context?

image

#define HARVEST_REQUEST_PARAM_APPEND(string, key, value, edited, ampersand)                        \
	do {                                                                                           \
		(if (edited) {                                                                             \
			if (ampersand) {                                                                       \
				g_string_append_printf(string, "&%s=%s", #key, #value);                            \
			} else {                                                                               \
				ampersand = TRUE;                                                                  \
				g_string_append_printf(string, "%s=%s", #key, #value);                             \
			}                                                                                      \
		})                                                                                         \
	} while (0)

Originally posted by @tristan957 in #311 (comment)

@matter123
Copy link
Collaborator

matter123 commented Jul 24, 2019

@jeff-hykin This is apparently a valid macro function start. Do you want to try to support it?

#define foo(arg1\
, arg2, arg3)

In use: https://godbolt.org/z/ylRBsC

@matter123
Copy link
Collaborator

@tristan957 is this a .c file? If so TRUE should be recognized as a boolean value.

@tristan957
Copy link
Author

Yes it is

@matter123
Copy link
Collaborator

Huh, when I use C syntax highlighting. TRUE has the scope constant.language.c.
(v1.12.20)
Screenshot from 2019-07-24 12-23-38

Header have by default C++ syntax highlighting, If it is not a header, however, I am not sure what the issue might be.

@matter123 matter123 added Syntax: C For bugs that only occur in C Syntax: C++ issue exists for C++ labels Jul 24, 2019
@tristan957
Copy link
Author

It was a .h file actually. Header files in my projects are C files. I can check the scopes on my end later today.

@matter123
Copy link
Collaborator

matter123 commented Jul 24, 2019

Unfortunately without something like the C/C++ extension or Modelines to changes the syntax on a per-file basis, all .h files end up with c++ syntax highlighting. This is because both C and C++ declare support for the extension .h and it would seem, that c++ being later lexicographically,
wins.

@tristan957
Copy link
Author

tristan957 commented Jul 24, 2019

I have the C/C++ extension installed. One of my favorite extensions. I had disabled semantic highlighting when I made this issue however. Does that have a side-effect with regard to TRUE?

@matter123
Copy link
Collaborator

No, I was under the impression that it adjusted the file association for each file it looks at, but it seems that's just to mark extensionless files as cpp files.

To get C syntax highlighting for your .h files you should be able to, in your workspace settings, add

  "files.associations": {
	"*.h": "c"
  }

And .h files should have the correct syntax highlighting.

@tristan957
Copy link
Author

Ok then I guess I will do that when I am in that project next. I'll report if TRUE still doesn't report correctly.

@jeff-hykin
Copy link
Owner

There's actually several other syntax issues in the example.

First are the {'s, but I actually think those are a code issue since (if () { std::cout << "hi"; }); doesn't compile. However, after removing the ()'s there's still a problem with the }'s which is strange because it is colored correctly and both C and C++ seem to be using the same pattern for the {}'s.

@matter123 I'm not exactly sure what the cause is, it appears that all of the ranges are ending after a single line but starting the macro with class { fixes all of them.

Then the last problem is that the #key and #value are not highlighted. @matter123 the only surefire way I can see for highlighting them is adding them as includes when duplicating all of the ranges for the bailout pattern. A quick fix would be adding them to the :ever_present_context

@jeff-hykin
Copy link
Owner

@jeff-hykin This is apparently a valid macro function start. Do you want to try to support it?

#define foo(arg1\
, arg2, arg3)

In use: https://godbolt.org/z/ylRBsC

Right now I don't think so, but eventually yeah. We can drop the one-liner backreferences the legacy patterns are using and use a pattern range with a lookbehind for #define or a \G to make sure it only matches the beginning.

@matter123
Copy link
Collaborator

@jeff-hykin What is the issue with the {?

Wouldn't adding macro_argument to ever_present_context recreate #292?

@jeff-hykin
Copy link
Owner

@jeff-hykin What is the issue with the {?

It wasn't highlighted because it doesn't compile (because it's wrapped inside of ()'s) which is the correct behavior.

@tristan957
Copy link
Author

@matter123 the TRUE problem was fixed. Thank you my friend

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Syntax: C For bugs that only occur in C Syntax: C++ issue exists for C++
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants