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

rollback sql-hint and rewrite schema autocomplete #1722

Merged
merged 7 commits into from Oct 6, 2023
Merged

Conversation

azmy60
Copy link
Contributor

@azmy60 azmy60 commented Sep 30, 2023

I didn't realize we could autocomplete the schema with the original sql-hint file.

sql-hint is now reverted to this commit.

todos:

  • fork tests from cm5 repo, add additional tests such as:
    • must be able to autocomplete alias when using quotes on the table (e.g. select t.<column> from "MyTable" as t)
  • fix text/x-pgsql mode for postgres to allow " (double quotes) instead of ` (backticks) on identifiers or use test/x-sqlite mode temporarily (look at codemirror5/mode/sql/sql.js)
  • in postgres, alias shouldn't be quoted. The fix is not from sql-hint, but it's possible?, also look at codemirror5/mode/sql/sql.js

closes #1720

@azmy60 azmy60 marked this pull request as draft September 30, 2023 10:01
@azmy60
Copy link
Contributor Author

azmy60 commented Sep 30, 2023

This was tested using postgres. I'm gonna need to test other databases and some edge cases too. I'm expecting there is not gonna be a lot of other changes after this.

@azmy60 azmy60 marked this pull request as ready for review October 2, 2023 23:18
@azmy60 azmy60 changed the title rollback sql-hint to previous commit and rewrite schema autocomplete rollback sql-hint and rewrite schema autocomplete Oct 3, 2023
@rathboma
Copy link
Collaborator

rathboma commented Oct 6, 2023

@azmy60 what's the status of this PR, is it ready for review again? (only 3/4 tasks are marked as 'done')

@azmy60
Copy link
Contributor Author

azmy60 commented Oct 6, 2023

Oh sorry, I forgot to tell you I couldn't find the solution to the 4th one yet. It is not from the autocomplete.

But it's ready for review if we want to ignore that for now 😆.

Note: it happens only in postgres (using "text/x-pgsql" mode). So it must have the deal with the mode module, not the autocomplete or sql-hint.

@rathboma rathboma merged commit 7e889e8 into master Oct 6, 2023
4 checks passed
@rathboma rathboma deleted the fix/1721 branch October 6, 2023 23:42
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.

BUG: Autocomplete wiping whole line
2 participants