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

Revert "Fix symbol regex and format json file" #11

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

trendzetter
Copy link
Contributor

@trendzetter trendzetter commented Feb 19, 2024

Reverts #10

The change is just not needed, it lowers the readability of the regex changed and the way it was applied -mixing formatting with attempts at fixing something- is bad practice with regard to change management. If there is a "distinct behavior", which I couldn't identify so far, it should be possible to describe it.

"patterns": [
{
"begin": "'",
"end": "(?=\\s|[],:\"'.)])",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This regex doesn't seem correct to me, it has a closing parenthesis to many. Did you intend the closing parenthesis to be escaped?

Copy link
Contributor Author

@trendzetter trendzetter Jun 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@EnoF it is correct.
"You can include an unescaped closing bracket by placing it right after the opening bracket, or right after the negating caret. []x] matches a closing bracket or an x." source: https://www.regular-expressions.info/charclass.html
It's preferable to use the specific syntax that avoids unwieldy escape sequences.

Copy link
Contributor Author

@trendzetter trendzetter Jun 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See also:
#3
The Pact extension for Atom had initially the same issue, let me refer this earlier exchanges:
kadena-io/pact-atom#9

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The ultimate review of the actual functionality is of course building the extension at the specific point commit using VSCE command and install it locally in VS Code.

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

Successfully merging this pull request may close these issues.

3 participants