Skip to content

fix: improve operator detection and fix unknown segment #193

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

Merged
merged 2 commits into from
Jul 2, 2024

Conversation

wkeese
Copy link
Contributor

@wkeese wkeese commented Apr 6, 2024

This fixes the "special" segments to include some missing multi-character operators from https://www.w3schools.com/sql/sql_operators.asp and other sites.

It also expands the "special" segment to match any single character that doesn't match any of the other regexps. That's nice because it gets rid of the need for the "unknown" segment, avoiding the problem listed in #178.

This PR contains two commits that can be merged individually.

Fixes #150, #178, refs #148.

@wkeese wkeese mentioned this pull request Apr 6, 2024
@wkeese
Copy link
Contributor Author

wkeese commented Apr 6, 2024

Note that the ci error below seems unrelated to this PR, I don't know anything about @semantic-release/changelog.

Run npm ci
npm ERR! Cannot read property '@semantic-release/changelog' of undefined

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/runner/.npm/_logs/202[4](https://github.com/scriptcoded/sql-highlight/actions/runs/8584413208/job/23524645442?pr=193#step:4:5)-04-06T23_12_39_064Z-debug.log

@wkeese wkeese mentioned this pull request Apr 13, 2024
@wkeese wkeese force-pushed the fix/operator-parsing branch from 4a997ab to 820029a Compare April 14, 2024 01:31
@wkeese wkeese changed the title fix: improve operator detection fix: improve operator detection and fix unknown segment Apr 14, 2024
@wkeese wkeese force-pushed the fix/operator-parsing branch 2 times, most recently from 0634d9b to 47884ee Compare April 14, 2024 01:34
@scriptsbot
Copy link
Collaborator

This pull request has been marked as stale because it has been open for 14 days with no activity. Remove the stale label or comment or this will be closed in 5 days.

@wkeese
Copy link
Contributor Author

wkeese commented Apr 29, 2024

The PR is ready, the build error is an setup problem with github.

@scriptsbot scriptsbot removed the Stale label Apr 30, 2024
@scriptsbot
Copy link
Collaborator

This pull request has been marked as stale because it has been open for 14 days with no activity. Remove the stale label or comment or this will be closed in 5 days.

@wkeese
Copy link
Contributor Author

wkeese commented May 14, 2024

The PR is ready, the build error is an setup problem with github.

@scriptsbot scriptsbot removed the Stale label May 15, 2024
@scriptsbot
Copy link
Collaborator

This pull request has been marked as stale because it has been open for 14 days with no activity. Remove the stale label or comment or this will be closed in 5 days.

@scriptsbot
Copy link
Collaborator

This pull request was closed because it has been stalled for 5 days with no activity.

@scriptsbot scriptsbot closed this Jun 4, 2024
@scriptcoded
Copy link
Owner

scriptcoded commented Jul 2, 2024

Hi @wkeese. I'm so sorry for not touching on this and letting the bot harass you. The last few months during which I've neglected maintenance has left this project in a bit of a sorry state. I think the lack of maintenance combined with the unresolved braces CVE in the master branch caused a drop in usage, and now my try to clean up some workflows last week resulted in an accidental 5.0.0 release on NPM which cannot be recalled any longer.

This looks good, as it usually does with your code. I'll merge this into beta and ensure it's stable and aim to get a 5.1.0 release out ASAP, since the 5.0.0 isn't stable enough to be called a proper release.

As always, thank you.

Edit: I'll merge this regardless of the failing Node 14 test as it won't affect the beta branch which drops support for Node 14.

@scriptcoded scriptcoded reopened this Jul 2, 2024
wkeese added 2 commits July 2, 2024 22:35
This fixes the "special" segments to include some missing operators from
https://www.w3schools.com/sql/sql_operators.asp and other sites.

I took the conservative approach of listing all the operators, as opposed to
the general regexp /(?<special>[^\w\s"'`]+)/, because the conservative approach
works better in certain cases such as "x>-5".  The downside is that you need
to list all the operators, so it's a bit fragile, especially since the exact
operators vary by SQL version (MySQL, TransactSQL, Postgres, etc.).

I kept all the symbols previously specified as "special" even though some
of them aren't operators, specifically: , ; : .

Fixes scriptcoded#150
Rearrange regexps so "special" regexp catches everything not caught by other regexps.
Eliminates the "unknown" segment I added in scriptcoded#148.

In practice, the only time we would hit the "unknown" segment is for a few weird
characters that weren't already caught by the "special" segment, for example, the
? and unclosed ` in "a `?> b".  I figured that in those rare cases, we might as
well just call those characters "special".

Fixes scriptcoded#178, refs scriptcoded#148.
@scriptcoded scriptcoded force-pushed the fix/operator-parsing branch from 47884ee to db31b60 Compare July 2, 2024 20:35
@scriptcoded
Copy link
Owner

Turns out rebasing the PR fixed the tests as well.

@scriptcoded scriptcoded merged commit 07b58be into scriptcoded:beta Jul 2, 2024
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants