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

Adds lang service switch #18034

Closed

Conversation

lewis-sanchez
Copy link
Contributor

@lewis-sanchez lewis-sanchez commented Sep 10, 2024

This PR fixes #17298

This PR adds a new setting to the MSSQL extension that allows users to have the language mode set to none by default
image

New query editors that are opened when this setting is enabled, will have "None" as the chosen language
image

Previously saved query files that are opened when this setting is enabled, will also have "None" as the chosen language
image

To enable the MSSQL language mode for an editor, first click on "None" in the status bar, and then choose MSSQL as the language
image

Copy link

VSIX Size Comparison

  • Main branch VSIX size: 11750 KB
  • PR branch VSIX size: 11750 KB
  • Size difference: $${\color{green} 0 KB \space (0\%) }$$

React Webview Bundle Size Comparison

  • Main branch bundle size: 2416 KB
  • PR branch bundle size: 2416 KB
  • Size difference: $${\color{green} 0 KB \space (0\%) }$$

@aasimkhan30
Copy link
Contributor

aasimkhan30 commented Sep 10, 2024

Maybe we need some PM feedback on this.

Currently this option sets intellisense language to 'none' for all sql editors.

However, there are 2 conditions which should always have 'mssql' intellisense enabled.

  1. if we open a query editor from a OE context menu, it should launch as MSSQL editor always.
    image
  2. if user specifically opens an MSSQL editor from command line, it should still have the language set to MSSQL.
    image

Maybe this option should only apply to .sql files opened from disk as those files can belong to any database provider (PGSQL, MYSQL....)

@lewis-sanchez
Copy link
Contributor Author

@aasimkhan30, here's another GitHub issue that is related to the concern you're pointing out. (#801).

This PR was intended to add a switch to optionally set the language mode to "None" as the default, instead of MSSQL when opening editors. The new switch that is added by this PR is initially turned off, meaning that MSSQL will be the default language mode when editors are opened.

@kburtram
Copy link
Member

@aasimkhan30, here's another GitHub issue that is related to the concern you're pointing out. (#801).

This PR was intended to add a switch to optionally set the language mode to "None" as the default, instead of MSSQL when opening editors. The new switch that is added by this PR is initially turned off, meaning that MSSQL will be the default language mode when editors are opened.

How does #801 address the point Aasim mentions regarding opening editors with an MSSQL connection? I think in those scenarios the language mode should probably be MSSQL regardless of this setting. I think this setting is more relevant for non-connected SQL files.

Copy link
Member

@kburtram kburtram left a comment

Choose a reason for hiding this comment

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

I think we need to do a design review on Intellisense settings and how to handle connections for new SQL documents. And how best to handle non-MSSQL compatible SQL files. This seems like part of the solution but maybe should be a little different than currently implemented.

@Benjin Benjin self-requested a review September 16, 2024 21:19
@Benjin
Copy link
Contributor

Benjin commented Sep 19, 2024

Closing PR as we revisit the design

@Benjin Benjin closed this Sep 19, 2024
@croblesm croblesm self-assigned this Dec 5, 2024
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.

[Suggestion] Include an option to set MSSQL to none by default
5 participants