Skip to content

Port gather survey and icon getting disabled while gathering #13296

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

Merged
merged 2 commits into from
Aug 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
([#13218](https://github.com/Microsoft/vscode-python/issues/13218))
1. Use jupyter inspect to get signature of dynamic functions in notebook editor when language server doesn't provide enough hint.
([#13259](https://github.com/Microsoft/vscode-python/issues/13259))
1. The gather icon will change and get disabled while gather is executing.
([#13177](https://github.com/microsoft/vscode-python/issues/13177))

### Fixes

Expand All @@ -36,7 +38,7 @@
1. Don't loop selection through all failed tests every time tests are run.
([#11743](https://github.com/Microsoft/vscode-python/issues/11743))
1. Some tools (like pytest) rely on the existence of `sys.path[0]`, so
deleting it in the isolation script can sometimes cause problems. The
deleting it in the isolation script can sometimes cause problems. The
solution is to point `sys.path[0]` to a bogus directory that we know
does not exist (assuming noone modifies the extension install dir).
([#11875](https://github.com/Microsoft/vscode-python/issues/11875))
Expand All @@ -47,7 +49,7 @@
1. Make the data science variable explorer support high contrast color theme.
([#12766](https://github.com/Microsoft/vscode-python/issues/12766))
1. The change in PR #12795 led to one particular test suite to take longer
to run. Here we increase the timeout for that suite to get the test
to run. Here we increase the timeout for that suite to get the test
Copy link
Member

Choose a reason for hiding this comment

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

They look like just whitespace changes, but there seem to be more things different in the changelog? Were these changes in your PR to master?

Copy link
Author

Choose a reason for hiding this comment

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

No, its VS code and prettier 'correcting' stuff when I save.

passing.
([#12833](https://github.com/Microsoft/vscode-python/issues/12833))
1. Refactor data science filesystem usage to correctly handle files which are potentially remote.
Expand Down Expand Up @@ -6793,4 +6795,4 @@ the following people who contributed code:

## Version 0.0.3

- Added support for debugging using PDB
- Added support for debugging using PDB
14 changes: 7 additions & 7 deletions package.nls.json
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,7 @@
"DataScience.findJupyterCommandProgressCheckInterpreter": "Checking {0}.",
"DataScience.findJupyterCommandProgressSearchCurrentPath": "Searching current path.",
"DataScience.gatherError": "Gather internal error",
"DataScience.gatheredScriptDescription": "# This file was generated by the Gather Extension.\n# It requires version 2020.7.94776 (or newer) of the Python Extension.\n#\n# The intent is that it contains only the code required to produce\n# the same results as the cell originally selected for gathering.\n# Please note that the Python analysis is quite conservative, so if\n# it is unsure whether a line of code is necessary for execution, it\n# will err on the side of including it.\n#\n# Please let us know if you are satisfied with what was gathered here:\n# https://aka.ms/gathersurvey\n\n",
"DataScience.gatheredScriptDescription": "# This file was generated by the Gather Extension.\n# It requires version 2020.7.94776 (or newer) of the Python Extension.\n#\n# The intent is that it contains only the code required to produce\n# the same results as the cell originally selected for gathering.\n# Please note that the Python analysis is quite conservative, so if\n# it is unsure whether a line of code is necessary for execution, it\n# will err on the side of including it.\n#\n# Please let us know if you are satisfied with what was gathered here:\n# https://aka.ms/gatherfeedback\n\n",
"DataScience.gatheredNotebookDescriptionInMarkdown": "## Gathered Notebook\nGathered from ```{0}```\n\n| | |\n|---|---|\n|  &nbsp|This notebook was generated by the Gather Extension. It requires version 2020.7.94776 (or newer) of the Python Extension, please update [here](https://command:python.datascience.latestExtension). The intent is that it contains only the code and cells required to produce the same results as the cell originally selected for gathering. Please note that the Python analysis is quite conservative, so if it is unsure whether a line of code is necessary for execution, it will err on the side of including it.|\n\n**Are you satisfied with the code that was gathered?**\n\n[Yes](https://command:python.datascience.gatherquality?yes) [No](https://command:python.datascience.gatherquality?no)",
"DataScience.savePngTitle": "Save Image",
"DataScience.jupyterSelectURIQuickPickTitle": "Pick how to connect to Jupyter",
Expand Down Expand Up @@ -584,10 +584,10 @@
"DataScienceRendererExtension.downloadCompletedOutputMessage": "Notebook Renderers extension download complete.",
"DataScience.uriProviderDescriptionFormat": "{0} (From {1} extension)",
"DataScience.unknownPackage": "unknown",
"DataScience.interactiveWindowTitle" : "Python Interactive",
"DataScience.interactiveWindowTitleFormat" : "Python Interactive - {0}",
"DataScience.interactiveWindowModeBannerTitle" : "Do you want to open a new Python Interactive window for this file? [More Information](command:workbench.action.openSettings?%5B%22python.dataScience.interactiveWindowMode%22%5D).",
"DataScience.interactiveWindowModeBannerSwitchYes" : "Yes",
"DataScience.interactiveWindowModeBannerSwitchAlways" : "Always",
"DataScience.interactiveWindowModeBannerSwitchNo" : "No"
"DataScience.interactiveWindowTitle": "Python Interactive",
"DataScience.interactiveWindowTitleFormat": "Python Interactive - {0}",
"DataScience.interactiveWindowModeBannerTitle": "Do you want to open a new Python Interactive window for this file? [More Information](command:workbench.action.openSettings?%5B%22python.dataScience.interactiveWindowMode%22%5D).",
"DataScience.interactiveWindowModeBannerSwitchYes": "Yes",
"DataScience.interactiveWindowModeBannerSwitchAlways": "Always",
"DataScience.interactiveWindowModeBannerSwitchNo": "No"
}
2 changes: 1 addition & 1 deletion src/client/common/utils/localize.ts
Original file line number Diff line number Diff line change
Expand Up @@ -925,7 +925,7 @@ export namespace DataScience {
export const gatherError = localize('DataScience.gatherError', 'Gather internal error');
export const gatheredScriptDescription = localize(
'DataScience.gatheredScriptDescription',
'# This file was generated by the Gather Extension.\n# It requires version 2020.7.94776 (or newer) of the Python Extension.\n#\n# The intent is that it contains only the code required to produce\n# the same results as the cell originally selected for gathering.\n# Please note that the Python analysis is quite conservative, so if\n# it is unsure whether a line of code is necessary for execution, it\n# will err on the side of including it.\n#\n# Please let us know if you are satisfied with what was gathered here:\n# https://aka.ms/gathersurvey\n\n'
'# This file was generated by the Gather Extension.\n# It requires version 2020.7.94776 (or newer) of the Python Extension.\n#\n# The intent is that it contains only the code required to produce\n# the same results as the cell originally selected for gathering.\n# Please note that the Python analysis is quite conservative, so if\n# it is unsure whether a line of code is necessary for execution, it\n# will err on the side of including it.\n#\n# Please let us know if you are satisfied with what was gathered here:\n# https://aka.ms/gatherfeedback\n\n'
);
export const gatheredNotebookDescriptionInMarkdown = localize(
'DataScience.gatheredNotebookDescriptionInMarkdown',
Expand Down
2 changes: 1 addition & 1 deletion src/client/datascience/commands/commandRegistry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ export class CommandRegistry implements IDisposable {

private reportGatherQuality(val: string) {
sendTelemetryEvent(Telemetry.GatherQualityReport, undefined, { result: val[0] === 'no' ? 'no' : 'yes' });
env.openExternal(Uri.parse(`https://aka.ms/gathersurvey?succeed=${val[0]}`));
env.openExternal(Uri.parse(`https://aka.ms/gatherfeedback?succeed=${val[0]}`));
}

private openPythonExtensionPage() {
Expand Down
42 changes: 32 additions & 10 deletions src/client/datascience/gather/gatherListener.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,10 +74,18 @@ export class GatherListener implements IInteractiveWindowListener {
break;

case InteractiveWindowMessages.GatherCode:
this.postEmitter.fire({
message: InteractiveWindowMessages.Gathering,
payload: { cellId: payload.id, gathering: true }
});
this.handleMessage(message, payload, this.doGather);
break;

case InteractiveWindowMessages.GatherCodeToScript:
this.postEmitter.fire({
message: InteractiveWindowMessages.Gathering,
payload: { cellId: payload.id, gathering: true }
});
this.handleMessage(message, payload, this.doGatherToScript);
break;

Expand Down Expand Up @@ -142,18 +150,32 @@ export class GatherListener implements IInteractiveWindowListener {
}
}

private doGather(payload: ICell): void {
this.gatherCodeInternal(payload).catch((err) => {
traceError(`Gather to Notebook error: ${err}`);
this.applicationShell.showErrorMessage(err);
});
private doGather(payload: ICell): Promise<void> {
return this.gatherCodeInternal(payload)
.catch((err) => {
traceError(`Gather to Notebook error: ${err}`);
this.applicationShell.showErrorMessage(err);
})
.finally(() =>
this.postEmitter.fire({
message: InteractiveWindowMessages.Gathering,
payload: { cellId: payload.id, gathering: false }
})
);
}

private doGatherToScript(payload: ICell): void {
this.gatherCodeInternal(payload, true).catch((err) => {
traceError(`Gather to Script error: ${err}`);
this.applicationShell.showErrorMessage(err);
});
private doGatherToScript(payload: ICell): Promise<void> {
return this.gatherCodeInternal(payload, true)
.catch((err) => {
traceError(`Gather to Script error: ${err}`);
this.applicationShell.showErrorMessage(err);
})
.finally(() =>
this.postEmitter.fire({
message: InteractiveWindowMessages.Gathering,
payload: { cellId: payload.id, gathering: false }
})
);
}

private gatherCodeInternal = async (cell: ICell, toScript: boolean = false) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { DebugProtocol } from 'vscode-debugprotocol';
import {
CommonActionType,
IAddCellAction,
IChangeGatherStatus,
ILoadIPyWidgetClassFailureAction,
IVariableExplorerHeight,
LoadIPyWidgetClassLoadAction,
Expand Down Expand Up @@ -98,6 +99,7 @@ export enum InteractiveWindowMessages {
StopDebugging = 'stop_debugging',
GatherCode = 'gather_code',
GatherCodeToScript = 'gather_code_to_script',
Gathering = 'gathering',
LaunchNotebookTrustPrompt = 'launch_notebook_trust_prompt',
TrustNotebookComplete = 'trust_notebook_complete',
LoadAllCells = 'load_all_cells',
Expand Down Expand Up @@ -630,6 +632,7 @@ export class IInteractiveWindowMapping {
public [InteractiveWindowMessages.StopDebugging]: never | undefined;
public [InteractiveWindowMessages.GatherCode]: ICell;
public [InteractiveWindowMessages.GatherCodeToScript]: ICell;
public [InteractiveWindowMessages.Gathering]: IChangeGatherStatus;
public [InteractiveWindowMessages.LaunchNotebookTrustPrompt]: never | undefined;
public [InteractiveWindowMessages.TrustNotebookComplete]: never | undefined;
public [InteractiveWindowMessages.LoadAllCells]: ILoadAllCells;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ const messageWithMessageTypes: MessageMapping<IInteractiveWindowMapping> & Messa
[InteractiveWindowMessages.FocusedCellEditor]: MessageType.syncWithLiveShare,
[InteractiveWindowMessages.GatherCode]: MessageType.other,
[InteractiveWindowMessages.GatherCodeToScript]: MessageType.other,
[InteractiveWindowMessages.Gathering]: MessageType.other,
[InteractiveWindowMessages.GetAllCells]: MessageType.other,
[InteractiveWindowMessages.ForceVariableRefresh]: MessageType.other,
[InteractiveWindowMessages.GetVariablesRequest]: MessageType.other,
Expand Down
7 changes: 6 additions & 1 deletion src/datascience-ui/history-react/interactiveCell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -195,8 +195,13 @@ export class InteractiveCell extends React.Component<IInteractiveCellProps> {
!this.props.settings.gatherIsInstalled
}
tooltip={getLocString('DataScience.gatherCodeTooltip', 'Gather code')}
disabled={this.props.cellVM.gathering}
>
<Image baseTheme={this.props.baseTheme} class="image-button-image" image={ImageName.GatherCode} />
<Image
baseTheme={this.props.baseTheme}
class="image-button-image"
image={this.props.cellVM.gathering ? ImageName.Sync : ImageName.GatherCode}
/>
</ImageButton>
<ImageButton
baseTheme={this.props.baseTheme}
Expand Down
11 changes: 5 additions & 6 deletions src/datascience-ui/history-react/interactivePanel.less
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/* Import common styles and then override them below */
@import "../interactive-common/common.css";
@import '../interactive-common/common.css';

.toolbar-menu-bar-child {
background: var(--override-background, var(--vscode-editor-background));
Expand Down Expand Up @@ -37,27 +37,26 @@
}

.cell-outer {
display:grid;
display: grid;
grid-template-columns: auto minmax(0, 1fr) 8px;
grid-column-gap: 3px;
width: 100%;
}


.cell-output {
margin: 0px;
width: 100%;
overflow-x: scroll;
background: transparent;
}

.cell-output>div {
.cell-output > div {
background: var(--override-widget-background, var(--vscode-notifications-background));
}

xmp {
margin: 0px;
}
}

.cell-input {
margin: 0;
Expand All @@ -72,4 +71,4 @@ xmp {
white-space: pre-wrap;
word-break: break-all;
overflow-x: hidden;
}
}
3 changes: 2 additions & 1 deletion src/datascience-ui/history-react/redux/reducers/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,5 +71,6 @@ export const reducerMap: Partial<IInteractiveActionMapping> = {
[InteractiveWindowMessages.UpdateKernel]: Kernel.updateStatus,
[SharedMessages.LocInit]: CommonEffects.handleLocInit,
[InteractiveWindowMessages.UpdateDisplayData]: CommonEffects.handleUpdateDisplayData,
[InteractiveWindowMessages.HasCell]: Transfer.hasCell
[InteractiveWindowMessages.HasCell]: Transfer.hasCell,
[InteractiveWindowMessages.Gathering]: Transfer.gathering
};
Loading