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

Fix: Leave aliases to be unquoted in autocomplete #1740

Merged
merged 6 commits into from Dec 4, 2023
Merged

Conversation

azmy60
Copy link
Contributor

@azmy60 azmy60 commented Oct 14, 2023

Testing CodeMirror outside of bks just little bit. It turns out that this behavior was not from the CodeMirror itself😅 .

Fixes point 2 in #1721

Copy link
Contributor

@not-night-but not-night-but left a comment

Choose a reason for hiding this comment

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

@azmy60 Nice! Glad we got this one figured out.

Code looks good, but do you think you could add some regression tests for this?

@azmy60
Copy link
Contributor Author

azmy60 commented Oct 14, 2023

Oh yeah, forgot that one. Let me see if I can do it.

@azmy60 azmy60 changed the title Leave aliases to be unquoted in autocomplete Fix: Leave aliases to be unquoted in autocomplete Oct 14, 2023
Copy link
Collaborator

@rathboma rathboma left a comment

Choose a reason for hiding this comment

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

Looks good, question: is this a problem in other dialects?

apps/studio/src/lib/codemirror.ts Show resolved Hide resolved
@rathboma
Copy link
Collaborator

rathboma commented Nov 8, 2023

Ok for you to merge, but if also a problem in other dialects, they'll need sorting also. eg SQL Server has schemas + tables also

Copy link
Collaborator

@rathboma rathboma left a comment

Choose a reason for hiding this comment

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

Feel free to merge this, but check my question first!

@azmy60
Copy link
Contributor Author

azmy60 commented Nov 27, 2023

Redshift needs the autoquote too so that's added. Merging now :) Everything else looks good!

@azmy60
Copy link
Contributor Author

azmy60 commented Nov 27, 2023

hmm.. I can't merge it somehow 🤔

@rathboma rathboma merged commit 93ed237 into master Dec 4, 2023
4 checks passed
@rathboma rathboma deleted the fix/quoted-alias branch December 4, 2023 21:28
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.

None yet

3 participants