Skip to content

Add max length configuration for connection info in status bar #19276

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

bathetrade
Copy link

Allows configuring the display width of the connection info in the status bar.

This makes it possible to extend the width and see the entire server name, database name, and user name without needing to hover over the item with the mouse. This in turn reduces the likelihood that a query is issued to the wrong database and may be more familiar for SSMS users where such information is always displayed in the title bar.

@bathetrade
Copy link
Author

@microsoft-github-policy-service agree

@kburtram
Copy link
Member

kburtram commented May 8, 2025

@Benjin what do you think of this one? It looks fine to me. Though maybe the default should be -1, or 100. Or maybe 50 is enough for most people?

@bathetrade
Copy link
Author

@kburtram The default width before this PR was 50 (Constants.maxDisplayedStatusTextLength), so I just left it at that. The servers, databases, and usernames I regularly work with however aren't particularly long and are truncated enough that a good amount of the database name is hidden, so I imagine increasing the default would be a welcome change.

@Benjin
Copy link
Contributor

Benjin commented May 9, 2025

@kburtram agreed; this is a nice addition! I left one minor comment, then I'd be happy to take it.

@bathetrade bathetrade requested a review from Benjin May 10, 2025 23:31
@Benjin
Copy link
Contributor

Benjin commented May 30, 2025

Sorry for taking a while to circle back to this; @bathetrade, can you resolve the conflicts? Then I'll be ready to merge it in :)

@bathetrade
Copy link
Author

@Benjin I resolved the merge conflicts and made a couple more changes:

  1. Removed nesting of connectionInfo test suites since the "getConnectionDisplayString" suite wasn't running or showing up in the test explorer for me
  2. Moved VS Code configuration loading out of getConnectionDisplayString to make it easier to test and more flexible for callers
  3. Updated the README to reflect the new setting
  4. Changed the default length to -1 instead of 100 in light of Database Name Truncation in status bar leading to mistakes/ambiguity #19485 and Feature Request: Color Coding for DB Connections -- Clearer Indication of what db you are working in. #18865

While testing, I ran into the following issues (unrelated to this PR)

  1. Debug with "Launch Extension (With Other Extensions Disabled)"
  2. Open a new file and change the type to SQL
  3. Press ctrl+shift+c to open the connection picker and select a profile
  4. image
  5. Open the extension settings from the extension's cogwheel in the sidebar
  6. Change a setting (I used the new status bar setting, but others seem to have the same behavior)
  7. Click somewhere outside of the setting area to commit the change
  8. image

I tested off main and was getting the same behavior but wanted to mention it anyway in case it's not yet being tracked.

Another issue is that the display string is now using icon tokens such as "$(database)". If you specify a max length that lands in the middle of such a token, the user will see something like "myServer: $(data ..." in the status bar. I didn't have time to fix it but would be happy to at some point if deemed worthwhile.

@swaschitz
Copy link

As an update since this feature/change doesn't appear done, with the release of 1.33.0, the default behavior has actually gotten worse. It now truncates earlier.

Image

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.

4 participants