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

PCRE2 broke assign and for_windows #6886

Closed
Billli11 opened this issue Mar 12, 2022 · 8 comments · Fixed by #6892
Closed

PCRE2 broke assign and for_windows #6886

Billli11 opened this issue Mar 12, 2022 · 8 comments · Fixed by #6892
Labels
bug Not working as intended

Comments

@Billli11
Copy link
Contributor

Billli11 commented Mar 12, 2022

Please fill out the following:

  • Sway Version:

broken version
sway version 1.8-dev-f614f35e (Mar 12 2022, branch 'master')

working version
sway version 1.8-dev-04676936 (Mar 12 2022, branch 'master')

installed via arch AUR sway-git

The latest commit broke assign and for_windows .
The command now do nothing.

sway seem to accept the message but when opening the windows with matching criteria nothing happen.

00:00:40.849 [INFO] [sway/commands.c:258] Handling command 'for_window [title="^xeyes$"] floating enable'
00:00:40.849 [DEBUG] [sway/criteria.c:718] Found pair: title=^xeyes$
00:00:40.849 [DEBUG] [sway/commands/for_window.c:34] for_window: '[title="^xeyes$"]' -> 'floating enable' added
00:00:40.849 [DEBUG] [sway/ipc-server.c:959] Added IPC reply of type 0x0 to client 85 queue: [ { "success": true } ]

here is some of the command tried.

for_window [title="^xeyes$"] floating enable
for_window [title=^xeyes$] floating enable
for_window [title="xeyes"] floating enable
assign [app_id="^com.obsproject.Studio$"] $ws5
assign [class="^discord$"] $ws5

those command is working when built without this commit

@Billli11 Billli11 added the bug Not working as intended label Mar 12, 2022
@emersion
Copy link
Member

emersion commented Mar 12, 2022

cc @ndren, ref #6883

@emersion emersion changed the title PR #6883 broke assign and for_windows PCRE2 broke assign and for_windows Mar 13, 2022
@Scrumplex
Copy link
Contributor

Scrumplex commented Mar 13, 2022

I have written this code to compare pcre and pcre2 (grabbed the respective code from Sway):

#include <string.h>
#include <stdio.h>
#include <stdlib.h>
#include <stdbool.h>

#define PCRE2_CODE_UNIT_WIDTH 8
#include <pcre2.h>
#include <pcre.h>

static bool generate_regex(pcre **regex, char *value) {
	const char *reg_err;
	int offset;

	*regex = pcre_compile(value, PCRE_UTF8 | PCRE_UCP, &reg_err, &offset, NULL);

	if (!*regex) {
		printf("Regex compilation for '%s' failed: %s", value, reg_err);
		return false;
	}
    
	return true;
}

static bool generate_regex2(pcre2_code **regex, char *value) {
	int errorcode;
	PCRE2_SIZE offset;

	*regex = pcre2_compile((PCRE2_SPTR)value, PCRE2_ZERO_TERMINATED, PCRE2_UTF | PCRE2_UCP, &errorcode, &offset, NULL);
	if (!*regex) {
		PCRE2_UCHAR buffer[256];
		pcre2_get_error_message(errorcode, buffer, sizeof(buffer));

		printf("Regex compilation for '%s' failed: %s", value, buffer);
		return false;
	}

	return true;
}

static int regex_cmp(const char *item, const pcre *regex) {
	return pcre_exec(regex, NULL, item, strlen(item), 0, 0, NULL, 0);
}

static int regex_cmp2(const char *item, const pcre2_code *regex) {
	pcre2_match_data *match_data = pcre2_match_data_create_from_pattern(regex, NULL);
	int result = pcre2_match(regex, (PCRE2_SPTR)item, strlen(item), 0, 0, match_data, NULL);
	pcre2_match_data_free(match_data);
	return result;
}

int main() {
    pcre *regex;

    generate_regex(&regex, "discord");
    int a1 = regex_cmp("discord", regex);

    pcre2_code *regex2;

    generate_regex2(&regex2, "discord");
    int a2 = regex_cmp2("discord", regex2);

    printf("%d %d", a1, a2);
}
$ gcc pcre.c -l pcre -l pcre2-posix -l pcre2-8
$ ./a.out
0 1

@markstos
Copy link
Contributor

Should the PCRE2 merge be backed out until this is resolved?

@Billli11
Copy link
Contributor Author

Seem like pcre2 will always return value >= 1 if matched.

From pcre2_match man page

The return from pcre2_match() is one more than the highest numbered capturing pair that has been set (for example, 1 if there are no captures), zero if the vector of offsets is too small, or a negative error code for no match and other errors.

And from pcre_exec man page

It returns offsets to captured substrings.

Trying "dis(cor)d" with pcre2 returned 2 instead of 1 and pcre is still returning 0.
And both will return -1 if there are no matching string.

@KogasaPls
Copy link

KogasaPls commented Mar 13, 2022

Related, I get "no matching node" when trying to match against marks (e.g. swaymsg [con_mark="test"] kill) even when they exist

@Billli11
Copy link
Contributor Author

Billli11 commented Mar 13, 2022

Changing
if (regex_cmp(title, criteria->title->regex) != 0) {
and
if (regex_cmp(con->marks->items[i], criteria->con_mark->regex) == 0) {
to
if (regex_cmp(title, criteria->title->regex) < 0) {
and
if (regex_cmp(con->marks->items[i], criteria->con_mark->regex) >= 0) {
in criteria.c
seem to fixed the problem.

tested with
for_window [title=xeyes] floating enable
assign [title=xeyes] 2
using default config file

Here is the diff

@ErikReider
Copy link
Contributor

@Billli11 Your patch works on my end :)

@emersion
Copy link
Member

@Billli11 can you submit a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Not working as intended
Development

Successfully merging a pull request may close this issue.

6 participants