* Move to using notebook language by default, with override in cell
* Update cell language on kernel change
* Tweak language logic so that it prefers code mirror mode, then falls back since this was failing some notebooks
* Add new package.json contribution to define language magics. These result in cell language changing. Language is cleared out on removing the language magic
* Added support for executing Python, R and Java in the SQL Kernel to prove this out. It converts to the sp_execute_external_script format
TODO in future PR:
* Need to hook up completion item support for magics (issue #4078)
* Should add indicator at the bottom of a cell when an alternate language has been detected (issue #4079)
* On executing Python, R or Java, should add some output showing the generated code (issue #4080)
* Updated version
* On building found it had build break due to unused imports. Turns out none of the code is used (it's just a package.json + SQL file extension) so removed it all
* Removed all unnecessary node module imports, reducing size from 5.4MB to 86KB. We should probably do this for all extensions
* Fix error where kernel name was compared to itself. This doesn't break anything right now since we happen to have special handling of Python3, and for other kernels they share the same set of supported providers (which is what the check is used for). However it needs fixing for next release.
* Fix console error due to queryTextEditor trying to access model before it's ready
* Fix issues where notebook model hooked to session events multiple times
* Removed calls that weren't needed such as loadActiveContexts (if undefined, does nothing) and passing connection to initialize method
- Fix#3844
- Fix#3955
- Specs can be null on early load of Jupyter kernels
- Cells were missing in some reference test .ipynb files. We should be resilient to malformed files if possible.
Fixes#3843. Now includes full fix which limits length and ensures a scrollbar is available
- Set max size for editor. 4000px gets us 200-250 lines before needing a scrollbar.
- Adds layout updating which should also ensure accurage line highlighting to the right of the editor. What's happening is initial size is slightly off, so need to layout a 2nd time (e.g. layout once, let flex figure things out, then layout a 2nd time). This isn't optimal as there's a minor perf hit but it isn't noticeable overall.
To consider in future PRs:
- Add user configurable setting for max length?
- Handle case where we scroll to bottom but scrollbar is at the top.
- Consider how intellisense will work on this. We may need to split into a window around the current code when sending to the kernel as it's quite likely that doing a 12K line intellisense request will be too big.
* Fix#3985 Hide cell toolbar for markdown cells
* Note that I'm still hiding the overall toolbar section per UX feedback
* Also now hiding line numbers per UX feedback..
- This is a recommended update, let me know if you have concerns
- Updated all samples and Azure Data Studio specific extensions with lodash dependency
Fixes#3931
- Align run button correctly so it's centered in new cell
- Refactor to support multi-state button.
- Hidden state is set to show execution count
- Stopped state shows run button
- Running state shows stop button
- Error state (will) show error button. This isn't fully handled right now
- Add execution count to model and to SqlKernel, verify serialization, loading, update matches other notebook viewers
**Notes on implementation**:
I think this is a decent solution for a) showing execution count as text, and b) perfectly centering the run button.
The below solution shows count correctly up to 999 runs (that’s clicking 999 times in a single session), the icon lines up juuust about right with [ ] but for other numbers it is pretty close but probably not exactly right. I wish I could solve this to work better but trying to change text float to center etc. really isn’t working.
**Screenshots**:
With running cell:
- Handles empty file scenario, with fixes along the way for missing metadata (bonus win)
- In non-empty file still shows error and kernel stuck in loading state. #3964 opened to track this issue and fix later
- Fixed so it's now invisible instead of empty when not selected.
- This fixes clickability and issue where it stayed visible in 1 fix
- Also fixed cell output action which used active cell instead of context cell.
This was reviewed / worked on with Smitha and will be signed off on by PM via mail.
1 thing left (make run button look better when not selected) will be one in separate review.
Changes
- Add top/bottom padding to editor so it's not cramped
- Added an (on by default) setting `notebook.overrideEditorTheming`. This controls whether new colors etc. are used for notebook editors or if users should see vanilla UI like in standard editor. Settings under this flag are:
- When unselected, editor has same color as toolbar. On selection it goes back to regular editor view so colors work "right"
- In standard light/dark themes we now use a filled in background color instead of border box.
* Added hover support, adding box shadow and light outline on hovering and the "more actions" button showing on hover
* Added box shadow for dark themes (hooray!)
* Remove border from everything but the code cell unless a cell is selected or hovered over. This ensures this looks like a document
* Fix high contrast theming issues.
- SQLKernel is the only place to listen for batch and query complete messages now
- It routes to the 1 and only future (since can only have 1 at a time
- It handles query cancelation and not-connected issues correctly
- Editor layout gets called sometimes when other events happen (and Notebook isn't visible)
- Add in a layout call on re-setting input so the cell is updated. This fixes the problem by laying out once the UI is visible again.
Note: long term, should really be destroying the UI (while preserving the model), then restoring it including scroll selection etc. and hooking back up to the model. That is... much more work, but something we'll need long term to avoid issues where we have many Notebooks open at once. Not in scope for this PR
- Toolbar background is now differentiated from the editor
- For unselected cells there's no longer a line selection in the cell. This makes it clearer what the active cell is (and cleans the UI up)
- Added `runCell` API. Updated runCell button to listen to events on the model so it'll reflect run cell when called from other sources
- Plumbed through kernelspec info to the extension side so when changed, it's updated
- Fixed bug in ConnectionProfile where it didn't copy from options but instead overrode with empty wrapper functions
Here's the rough test code (it's in the sql-vnext extension and will be out in a separate PR)
```ts
it('Should connect to local notebook server with result 2', async function() {
this.timeout(60000);
let pythonNotebook = Object.assign({}, expectedNotebookContent, { metadata: { kernelspec: { name: "python3", display_name: "Python 3" }}});
let uri = writeNotebookToFile(pythonNotebook);
await ensureJupyterInstalled();
let notebook = await sqlops.nb.showNotebookDocument(uri);
should(notebook.document.cells).have.length(1);
let ran = await notebook.runCell(notebook.document.cells[0]);
should(ran).be.true('Notebook runCell failed');
let cellOutputs = notebook.document.cells[0].contents.outputs;
should(cellOutputs).have.length(1);
let result = (<sqlops.nb.IExecuteResult>cellOutputs[0]).data['text/plain'];
should(result).equal('2');
try {
// TODO support closing the editor. Right now this prompts and there's no override for this. Need to fix in core
// Close the editor using the recommended vscode API
//await vscode.commands.executeCommand('workbench.action.closeActiveEditor');
}
catch (e) {}
});
it('Should connect to remote spark server with result 2', async function() {
this.timeout(240000);
let uri = writeNotebookToFile(expectedNotebookContent);
await ensureJupyterInstalled();
// Given a connection to a server exists
let connectionId = await connectToSparkIntegrationServer();
// When I open a Spark notebook and run the cell
let notebook = await sqlops.nb.showNotebookDocument(uri, {
connectionId: connectionId
});
should(notebook.document.cells).have.length(1);
let ran = await notebook.runCell(notebook.document.cells[0]);
should(ran).be.true('Notebook runCell failed');
// Then I expect to get the output result of 1+1, executed remotely against the Spark endpoint
let cellOutputs = notebook.document.cells[0].contents.outputs;
should(cellOutputs).have.length(4);
let sparkResult = (<sqlops.nb.IStreamResult>cellOutputs[3]).text;
should(sparkResult).equal('2');
try {
// TODO support closing the editor. Right now this prompts and there's no override for this. Need to fix in core
// Close the editor using the recommended vscode API
//await vscode.commands.executeCommand('workbench.action.closeActiveEditor');
}
catch (e) {}
});
});
```
* Refactor JSON and format files to model and fix tabs -> spaces issues
This is in prep for some work to reuse these code files inside the model,
so pushing as its own PR to keep the next piece of work clean.
* Fix#3680 Notebooks: outputs with string arrays rendered incorrectly
- Add support for processing v4 format files loaded from disk
- Prep support for v3 notebooks by adding placeholder code for that
* Fix failing tests and add specific one for this bug
* Remove references to v5
* Handle delayed Notebook provider registration
- Fixes#3197 Notebooks: builtin provider always used on reopen with notebook file visible
- Fixes#3414 Can't refresh kernel after connect to big data cluster
There are 3 parts to this fix:
- If no notebook provider other than the default is installed, we warn users and prompt to install the SQL2019 extension
- We wait on the extension host registration to complete before determining which provider to use
- We know that the extension registration of the provider instance will be after package.json is read, so if we wait after registration for 10 seconds to give this a chance to happen before returning a provider to the front end
* Remove launch.json change that was added accidentally
* Fix timeout not being the expected value
* Removed console log left in during debugging
* Remove unnecessary whitespace
* Fix unit test failure
* Name the registration better, and remove outdated comments
- Set markdown as language for markdown cell
- Fix issue where after loading language from cell metadata, always override it. This made the "language" feature irrelevant in the cell.
- Fixed tests with new behavior (assumption: cell-level language overrides notebook-level definition) and added new test to cover this too
- Add edit API that can be used in the extension
- Separated document and editor classes out since this is the point those get big. I can refactor back in if needed to ease code review
- Based this off text editing APIs but tweaked for the fact this is a cell/array based set of edits
initial support for Notebook extensibility. Fixes#3148 , Fixes#3382.
## Design notes
The extensibility patterns are modeled after the VSCode Document and Editor APIs but need to be different since core editor concepts are different - for example Notebooks have cells, and cells have contents rather than editors which have text lines.
Most importantly, a lot of the code is based on the MainThreadDocumentsAndEditors class, with some related classes (the MainThreadDocuments, and MainThreadEditors) brought in too. Given our current limitations I felt moving to add 3 full sets of extension host API classes was overkill so am currently using one. Will see if we need to change this in the future based on what we add in the additional APIs
## Limitations
The current implementation is limited to visible editors, rather than all documents in the workspace. We are not following the `openDocument` -> `showDocument` pattern, but instead just supporting `showDocument` directly.
## Changes in this PR
- Renamed existing APIs to make clear that they were about notebook contents, not about notebook behavior
- Added new APIs for querying notebook documents and editors
- Added new API for opening a notebook
- Moved `New Notebook` command to an extension, and added an `Open Notebook` command too
- Moved notebook feature flag to the extension
## Not covered in this PR
- Need to actually implement support for defining the provider and connection IDs for a notebook. this will be important to support New Notebook from a big data connection in Object Explorer
- Need to add APIs for adding cells, to support
- Need to implement the metadata for getting full notebook contents. I've only implemented to key APIs needed to make this all work.
Full plumb through of Session support. Also fixed some test issues
- Load session and get necessary information in kernels list
- Run Cell button now works as expected
- Added a ToggleAction base class which can be used for anything that switches icons. I'd still prefer to have this be dynamic and as clean as the extension classes
- Fixed account test unhandled promise rejections (caused by incorrect / invalid tests) that made it hard to see all the test run output.
* Support notebook file types contribution
- Extensions can define a provider and what file types it should be used for
- Verified that this works for Jupyter Content & Server Managers.
- Starts Jupyter server as expected
Not in this PR:
- Support for session manager end to end
- Tests
Implements provider contribution in the MainThreadNotebook, with matching function calls in the ExtHostNotebook class. This will allow us to proxy through notebook providers (specifically, creation of a notebook manager with required content, server managers) from an extension up through to the main process.
Implemented in this PR:
- Callthroughs for content and server manager APIs
- Very basic unit tests covering provider & manager registration
Not implemented:
- Fuller unit tests on the specific callthrough methods for content & server manager.
- Contribution point needed to test this (so we can actually pass through the extension's existing Notebook implementation)
- Fixed issues with sqlCreateTable snippet, which meant it couldn't effectively be used to tab through all fields and then hit execute without errors. Specifically fixed bugs where types like NVARCHAR were incorrectly escaped and the Id part of a column was outside the name section, both causing intellisense & execution breaks
- Changed SchemaName to dbo. This helps hit the "80% case" where objects are in the most common schema
- Removed DatabaseName from most snippets. The core issue is this requires you to manually type the exact database name into the snippet which is really hard. We should consider having separate "with Database" snippets or support SQLCMD variables which would let us default to the current database without needing users to manually type them in as alternatives, but having basic snippets just work on current DB is important.
- Defines a new NotebookService in Azure Data Studio which will be used to interact with notebooks. Since notebooks can require per-file instantiation the provider is just used to create & track managers for a given URI.
- Inject this into notebook.component.ts and pass required parameters that'll be used to properly initialize a manger into the method. Actual initialization not done yet.
- Port over & recompile notebook model code
- Define most required APIs in sqlops.proposed.d.ts. In the future, these will be used by extensions to contribute their own providers.
* Port the Azure Resource Explorer extension to core.
This will enable Azure viewlet by default in the next release.
- Moving this code from the SQL Server 2019 extension to Azure Data Studio core
- Ported tests and verified they work in the integration tests.sh file
- Fixed an issue that caused integration tests to fail if you have a SQL Server 2019 big data cluster endpoint listed, but the extension isn't installed.
The IdGenerator was recreated each time and had a high likelihood of conflicts. Invitably after adding dozens or hundreds of icons you'll start seeing the CSS class replaced and overridden.
The solution is to do like elsewhere: have 1 const that is loaded on first import of the file and keeps a global track.
Side note is that it'd be a good idea to cache CSS rules with the same iconPath so we don't create lots of additional rules unnecessarily. If we reuse the same icon a bunch we should cache them - #2524 is tracking this.
* Add support for clickable links and other webview options
- Added click handling
- Added suport for localResourceRoots to be read in the webview
* Options should not be mandatory
* Ensure the constructor-defined properties are preserved
- This fixes issue where the extensionFolderPath was lost during webview withProperties call in the modelbuilder.
* enableCommandUris should be a getter
* Add position support to webview and to flexContainer
* Fix regressions on editor view caused by merge
* Support editor content change notification, URI property and correctly align toolbar items
* Better fix for toolbar orientation, plus minor editor fixes
* Support position property in editor component
- This needs to be set directly on the editor component so cannot just use CSSStyles feature
- Given its importance to this scenario, it also warrants a dedicated property.
* Fix window resize when action bar clicked
* Renamed per Abbie's suggestion
* Changed name to clarify the comments
* Fixes#1856 Object Explorer needs Icons field for nodes separate from type/subtype
- Adds in the concept of a themeable icon path which matches VSCode's implementation. This should help support theme-based overrides in the future
- Fixes#1867 context menu should be extensible
- Added context keys to support "when" conditions on the new extensions
- Fixes issue where actions like New Query, scripting show up even if these are not valid for the provider type or object type
- Fixed node expansion bug where rapid connect / expand / disconnect could break the app (fix in ObjectExplorerService.onNodeExpanded)
- Major change to how internal actions work. These cannot assume the context has non-serializable objects. Opened up some APIs to make this easier to handle.
- Fixed a number of existing bugs in internal actions.
- Notably, DisconnectAction was adding a listener on each right-click on an active connection and never getting it disposed. This wasn't needed at all due to design changes.
- Another bug fix is that the Manage action now correctly navigates to the DB dashboard for database-level connections. Before this it went to the server-level dashboard.
* Define API for context info
- Fixes#1764, fixes#1765
- Fixes bug where configModifiers were passed in the dashboard service in some places, and a different object with incompatible / missing fields in a different one. Now it always passes in the owner object, and this should have the required fields
- Note: this doesn't fix the problem where the code does not fail to build, which I would have expected to be the case.
- Adds "provider" as an option for TabConfig including in the schema
- Filter by "provider" type when loading tabs. If provider is not set for a tab, will assume "MSSQL" since it's the default provider. This is a design decision - without this, we would either have to disable tabs that don't show this or have them incorrectly show for non-MSSQL provider types
Notes:
Does not override behavior for individual widgets, which still get the current provider set as their provider if not specified. This seems to be required so should be fine.
We will fix tasks/widgets that aren't supported by a provider showing up in a separate PR, since it's a different issue
- Fix a number of issues that arose while testing a provider without a metadata service or serverInfo object returned via DMP calls. These should be optional services/features and we should be resilient to them not existing. In most places we already have these checks
- This does not cover a number of "improvement" scenarios, such as filtering extension tabs by provider, and defaulting any tabs that don't specify a provider to be MSSQL. This and some other features to ensure things make sense will be implemented in separate PRs but this unblocked the scenario
This is needed to enable testing of webview-based experiences.
This was made an official API in the latest VSCode release, so on next merge this will be standard anyhow.
- Add the ActionDescriptor functionality. This is a table of information with actionable links
- Also add optional status indicator which shows a color. In the future, would like to extend to have icon in this space as an alternative.
- Fixed 1 issue with account management UI throwing an error on cancel
* Fix main layout issues and add box around card
- Need to improve box size as it's taking too much space
* Fix UI issues with flexContainer and cards
* Simplify card HTML
- Showing dashboard.tabs and dashboard.insights in Contributions
- Decided not to show dashboard.containers for now since these are logically just parts of the tabs, and dont' have user-facing benefit.
This is an initial PR for a new model-driven UI where extensions can provide definitions of the components & how they're laid out using Containers.
#1140, #1141, #1142, #1143 and #1144 are all tracking additional work needed to improve the initial implementation and fix some issues with the implementation.
Features:
- Supports defining a FlexContainer that maps to a flexbox-based layout.
- Supports creating a card component, which is a key-value pair based control that will lay out simple information to a user. Eventually this will have an optional set of actions associated with it.
- Has a sample project which shows how to use the API and was used for verification
VSCode refactored their terminal to have more delayed initialization,
but still return as complete before the actual instance is ready for focus
and paste events. The fix is to add a delay so this can complete before
calling the paste action. As this doesn't affect extensions (the hop to the
extension host causes a delay) I'm not raising this with VSCode as an
upstream issue, just fixing on our side
- Reenabled the link and added a click handler that actually opens up the help
- Changed to using an aka.ms link so that if the docs page changes in the future we can update the link and not need to re-ship the code
Kevin Cunnane
