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

Regex lookahead / lookbehind questions #163

Open
Danp2 opened this issue Jan 23, 2023 · 5 comments
Open

Regex lookahead / lookbehind questions #163

Danp2 opened this issue Jan 23, 2023 · 5 comments

Comments

@Danp2
Copy link

Danp2 commented Jan 23, 2023

I'll start off by stating that I am not a regex expert, so apologies if I'm looking at this incorrectly. There are several instances of regex used in this extension that I think need to be reviewed for correctness.

  • I don't understand what purpose (?=\S) serves in these instances. Can anyone clarify this for me?
  • Instead of a negative lookahead, shouldn't this be using a negative lookbehind for the comment detection?
    • IE (?<!;~\s) instead of (?!;~\s)
    • Removing the \s from the end also makes it function better IMO

Anyone have thoughts or comments on this topic? Below are the two lines of code --

const findFunc = /(?=\S)(?!;~\s)Func\s+((\w+)\((.+)?\))/i;

and

const includeFuncPattern = /(?=\S)(?!;~\s)Func\s+((\w+)\((.+)?\))/gi;

@Danp2
Copy link
Author

Danp2 commented Jan 30, 2023

image

@vanowm
Copy link

vanowm commented Mar 13, 2023

Hmmm that regex doesn't look right indeed.
It should be lookbehind, but only for ; character, no need match ~\s.
Also, it doesn't match functions with a space after its name.

There shouldn't be any non-whitespace characters in front of function, so this should be a better regex:

/^[\t ]*Func\s+(\w+)\s*\((.*)\)/im

@Danp2
Copy link
Author

Danp2 commented Mar 13, 2023

Hi @vanowm,

Thanks for taking a look at this. I will check out your PR in a bit.

Regards, Dan

@GrandStrateguerre
Copy link

hello,
Only detect the first function is it good ?

image

Best regards

@vanowm
Copy link

vanowm commented Mar 18, 2023

This depends on where this regex used in the code. Some places use g (global) flag:

const functionPattern = /^[\t ]*Func\s+(\w+)\s*\(/gim;

other use this regex per individual line:
const findFunc = /^[\t ]*Func\s+(\w+)\s*\((.*)\)/i;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants