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

Incorrect highlighting with C/C++ when macro call does not end in semicolon #311

Closed
tristan957 opened this issue Jul 15, 2019 · 16 comments
Closed
Labels
🐛 Bug Something isn't working

Comments

@tristan957
Copy link

I have reproduced this with C/C++ and on 1.12.20 and the default VSCode version.

#include <string.h>

#include <glib-object.h>
#include <gtk/gtk.h>

#include "hal-application.h"
#include "hal-config.h"
#include "hal-resources.h"
#include "hal-settings-dialog.h"
#include "hal-time-entry.h"
#include "hal-window.h"

struct _HalApplication
{
	GtkApplication parent_instance;
};

typedef struct HalApplicationPrivate
{
	HalWindow *main_window;

	GSettings *settings;
} HalApplicationPrivate;

// THIS LINE DOES NOT END IN A SEMICOLON BUT IS VALID C and C++
G_DEFINE_TYPE_WITH_PRIVATE(HalApplication, hal_application, GTK_TYPE_APPLICATION)

static void
hal_application_activate(GApplication *self)
{
	HalApplicationPrivate *priv = hal_application_get_instance_private(HAL_APPLICATION(self));

	if (priv->main_window == NULL) {
		priv->main_window = hal_window_new(self);
	}

	g_object_set(gtk_settings_get_default(), "gtk-application-prefer-dark-theme",
		g_settings_get_boolean(priv->settings, SETTINGS_PREFER_DARK_THEME), NULL);

	gtk_window_present(GTK_WINDOW(priv->main_window));
}

static void
hal_application_about(
	G_GNUC_UNUSED GSimpleAction *action, G_GNUC_UNUSED GVariant *param, gpointer data)
{
	static const char *authors[] = {"Tristan Partin"};

	HalApplication *self		= HAL_APPLICATION(data);
	HalApplicationPrivate *priv = hal_application_get_instance_private(self);

	gtk_show_about_dialog(GTK_WINDOW(priv->main_window), "program-name", PACKAGE_NAME, "version",
		PACKAGE_VERSION, "license-type", PACKAGE_LICENSE, "website", PACKAGE_WEBSITE,
		"website-label", PACKAGE_WEBSITE_LABEL, "authors", authors, "logo-icon-name", "trophy-gold",
		NULL);
}

static void
hal_application_settings(
	G_GNUC_UNUSED GSimpleAction *action, G_GNUC_UNUSED GVariant *param, gpointer data)
{
	HalApplication *self		= HAL_APPLICATION(data);
	HalApplicationPrivate *priv = hal_application_get_instance_private(self);

	HalSettingsDialog *dialog =
		hal_settings_dialog_new(GTK_WINDOW(priv->main_window), priv->settings);
	const GtkResponseType response = gtk_dialog_run(GTK_DIALOG(dialog));
	gtk_widget_destroy(GTK_WIDGET(dialog));

	if (response == GTK_RESPONSE_APPLY) {
		g_autofree gchar *harvest_api_key =
			g_settings_get_string(priv->settings, SETTINGS_HARVEST_API_KEY);
		GActionMap *map = G_ACTION_MAP(priv->main_window);
		if (strlen(harvest_api_key) != 0) {
			GAction *show_content = g_action_map_lookup_action(map, "show-content");
			g_action_activate(show_content, NULL);
		} else {
			GAction *show_content = g_action_map_lookup_action(map, "hide-content");
			g_action_activate(show_content, NULL);
		}
	}
}

static void
hal_application_time_entry_start(
	G_GNUC_UNUSED GSimpleAction *action, GVariant *param, gpointer data)
{
	const guint64 address			  = g_variant_get_uint64(param);
	G_GNUC_UNUSED HalTimeEntry *entry = HAL_TIME_ENTRY((HalTimeEntry *) address);

	g_autoptr(GNotification) notification = g_notification_new("Harvest Almanac");
	g_notification_set_body(notification, "Client -- Project timer started");
	g_notification_add_button_with_target(
		notification, "Stop Timer", "app.time-entry-stop", "t", address, NULL);
	g_application_send_notification(G_APPLICATION(data), "time-entry", notification);
}

static void
hal_application_time_entry_stop(G_GNUC_UNUSED GSimpleAction *action, GVariant *param, gpointer data)
{
	const guint64 address = g_variant_get_uint64(param);
	HalTimeEntry *entry   = HAL_TIME_ENTRY((HalTimeEntry *) address);

	hal_time_entry_stop(entry);
	g_application_withdraw_notification(G_APPLICATION(data), "time-entry");
}

static void
hal_application_startup(GApplication *self)
{
	g_resources_register(hal_get_resource());
	g_application_set_resource_base_path(self, "/io/partin/tristan/HarvestAlmanac");

	G_APPLICATION_CLASS(hal_application_parent_class)->startup(self);
}

static void
hal_application_finalize(GObject *obj)
{
	HalApplication *self		= HAL_APPLICATION(obj);
	HalApplicationPrivate *priv = hal_application_get_instance_private(self);

	g_clear_object(&priv->settings);

	G_OBJECT_CLASS(hal_application_parent_class)->finalize(obj);
}

static void
hal_application_class_init(HalApplicationClass *klass)
{
	GObjectClass *obj_class		 = G_OBJECT_CLASS(klass);
	GApplicationClass *app_class = G_APPLICATION_CLASS(klass);

	obj_class->finalize = hal_application_finalize;
	app_class->activate = hal_application_activate;
	app_class->startup  = hal_application_startup;
}

static const GActionEntry app_entries[] = {{.name = "about", .activate = hal_application_about},
	{.name = "settings", .activate = hal_application_settings},
	{.name				= "time-entry-start",
		.activate		= hal_application_time_entry_start,
		.parameter_type = "t"},
	{.name				= "time-entry-stop",
		.activate		= hal_application_time_entry_stop,
		.parameter_type = "t"}};

static void
hal_application_init(HalApplication *self)
{
	HalApplicationPrivate *priv = hal_application_get_instance_private(self);

	priv->settings = g_settings_new("io.partin.tristan.HarvestAlmanac");

	g_action_map_add_action_entries(
		G_ACTION_MAP(self), app_entries, G_N_ELEMENTS(app_entries), self);
}

HalApplication *
hal_application_new(const char *id)
{
	return g_object_new(HAL_TYPE_APPLICATION, "application-id", id, NULL);
}

Before semicolon
image

After semicolon
image

@tristan957
Copy link
Author

Even GitHub's syntax highlighting is broken lol

@tristan957
Copy link
Author

Teamwork makes the dream work.

@matter123 matter123 added the 🐛 Bug Something isn't working label Jul 15, 2019
@matter123
Copy link
Collaborator

matter123 commented Jul 15, 2019

I can't seem to reproduce. Can you confirm that the scopes are incorrect?

Screenshot from 2019-07-15 14-43-40

@tristan957
Copy link
Author

WTF. I can't reproduce on my Linux system. The above screenshot is from my mac. @matter123 I will get back to you tomorrow with the mac confirmation. Otherwise VSCode screwed me again.

@matter123
Copy link
Collaborator

matter123 commented Jul 16, 2019

Thanks for saying its mac specific. I'll confirm in a few minutes.

Edit: I am unable to reproduce.

Version: 1.37.0-insider
Commit: 7e03eb1a1fce3beb6ccea5f3b8f828e01940be61
Date: 2019-07-05T18:25:58.865Z
Electron: 4.2.5
Chrome: 69.0.3497.128
Node.js: 10.11.0
V8: 6.9.427.31-electron.0
OS: Darwin x64 18.6.0

@matter123
Copy link
Collaborator

Based on the color of the macro arguments. I am guessing that some code earlier in the file is the actual issue. If you can reproduce, the full scope for GTK_TYPE_APPLICATION would be helpful.

@tristan957
Copy link
Author

Sounds good. Thanks @matter123

@jeff-hykin
Copy link
Owner

I think this could be coming from the C++ extension. HalApplicationPrivate wouldn't be highlighted by the C or C++ grammar because we'd like to but we don't have the ability yet. The C++ extension just recently added this extra-coloring ability.

Trying to reproduce with all other extensions off will probably fix the OS-dependent differences. If it doesn't I really have no idea what is going on.

If the semicolon affects the validity of the code, that would affect the C++ extension's ability to add the additional highlighting. Right now the TextMate grammar is able to handle lots of wouldn't-compile code, but the C++ extension has a hard time with it.

@tristan957
Copy link
Author

@jeff-hykin that was a really good point. I turned on the enhanced highlighting with the last release of the C/C++ extension. Then I think this can be closed in favor of me opening another issue with that repository. Thanks for the debugging help.

It may help if you add a snippet to the issue template that people should check the syntax highlighting with "C_cpp.enhancedColorization" to "Disabled" to confirm that it is an issue with this extension.

@tristan957
Copy link
Author

@matter123 I don't want to spam another issue. Can you check the textmate scopes on these function pointers that GCC compiler extension attributes associated with them? They seem to lose their function pointer scope when the attributes are applied.

struct _HarvestRequestClass
{
	GObjectClass parent_class;

	const char *(*G_GNUC_CONST G_GNUC_WARN_UNUSED_RESULT serialize_params)(HarvestRequest *self);
	const char *(*G_GNUC_CONST get_endpoint)(HarvestRequest *self);
};

@matter123
Copy link
Collaborator

Macros that expand to an attribute are generally not supported as it is impossible to tell them apart from any other identifier.
Screenshot from 2019-07-22 22-47-19

@tristan957
Copy link
Author

That is what I figured. Thank you for the information!

@matter123
Copy link
Collaborator

G_GNU_[A-Z_]+ seems fairly common and mostly used for attributes. Maybe consider opening a new issue for that.

https://github.com/search?p=2&q=G_GNUC_&type=Code&utf8=%E2%9C%93

@tristan957
Copy link
Author

tristan957 commented Jul 23, 2019

It is a GLib thing. I am not totally convinced that you guys should start making exceptions in the grammar.

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)

@matter123
Copy link
Collaborator

matter123 commented Jul 23, 2019

So the comma issue is definitely a bug, the first 3 arguments are all one token. I don't think that TRUE is supposed to be recognized.

Open a new issue for the comma.

@tristan957
Copy link
Author

Will do

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants