Skip to content

Load and save variable filters #293

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.

Sign up for GitHub

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 3 commits into
base: main
Choose a base branch
from
Open

Load and save variable filters #293

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ea02bab
Load and save variable filters
kentarolim10 Dec 6, 2023
20b0bc6
Merge branch 'master' of https://github.com/kentarolim10/jupyterlab-v…
kentarolim10 Dec 7, 2023
1456fb2
Use promises for settings
kentarolim10 Dec 7, 2023
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
7 changes: 5 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,8 @@
"files": [
"lib/**/*.{d.ts,eot,gif,html,jpg,js,js.map,json,png,svg,woff2,ttf}",
"style/**/*.{css,eot,gif,html,jpg,json,png,svg,woff2,ttf}",
"style/index.js"
"style/index.js",
"schema/*.json"
],
"main": "lib/index.js",
"types": "lib/index.d.ts",
Expand Down Expand Up @@ -60,6 +61,7 @@
"@jupyterlab/outputarea": "^4.0.0",
"@jupyterlab/rendermime": "^4.0.0",
"@jupyterlab/services": "^7.0.0",
"@jupyterlab/settingregistry": "^4.0.9",
"@jupyterlab/ui-components": "^4.0.0",
"@lumino/coreutils": "^2.0.0",
"@lumino/datagrid": "^2.0.0",
Expand Down Expand Up @@ -95,7 +97,8 @@
},
"jupyterlab": {
"extension": true,
"outputDir": "lckr_jupyterlab_variableinspector/labextension"
"outputDir": "lckr_jupyterlab_variableinspector/labextension",
"schemaDir": "schema"
},
"styleModule": "style/index.js",
"eslintIgnore": [
Expand Down
19 changes: 19 additions & 0 deletions schema/plugin.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
{
"title": "Variable Inspector",
"description": "jupyterlab-variableinspector settings.",
"type": "object",
"properties": {
"filteredTypes": {
"type": "array",
"title": "Filter variables by type",
"description": "Filters out all variables with types matching the given filter",
"default": []
},
"filteredNames": {
"type": "array",
"title": "Filter variables by name",
"description": "Filters out all variables with names matching the given filter",
"default": []
}
}
}
21 changes: 15 additions & 6 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import { VariableInspectorManager } from './manager';
import { VariableInspectorPanel } from './variableinspector';

import { IVariableInspector, IVariableInspectorManager } from './tokens';
import { ISettingRegistry } from '@jupyterlab/settingregistry';

namespace CommandIDs {
export const open = 'variableinspector:open';
Expand All @@ -33,28 +34,36 @@ namespace CommandIDs {
* A service providing variable introspection.
*/
const variableinspector: JupyterFrontEndPlugin<IVariableInspectorManager> = {
id: '@lckr/jupyterlab_variableinspector',
requires: [ICommandPalette, ILayoutRestorer, ILabShell],
id: '@lckr/jupyterlab_variableinspector:plugin',
requires: [ICommandPalette, ILayoutRestorer, ILabShell, ISettingRegistry],
provides: IVariableInspectorManager,
autoStart: true,
activate: (
activate: async (
app: JupyterFrontEnd,
palette: ICommandPalette,
restorer: ILayoutRestorer,
labShell: ILabShell
): IVariableInspectorManager => {
labShell: ILabShell,
settingRegistry: ISettingRegistry
): Promise<IVariableInspectorManager> => {
const manager = new VariableInspectorManager();
const category = 'Variable Inspector';
const command = CommandIDs.open;
const label = 'Open Variable Inspector';
const namespace = 'variableinspector';
const tracker = new WidgetTracker<VariableInspectorPanel>({ namespace });
let settings: ISettingRegistry.ISettings;

try {
settings = await settingRegistry.load(variableinspector.id);
} catch (error) {
console.error('Failed to load settings for the Git Extension', error);
}
Copy link
Member

@fcollonval fcollonval Dec 7, 2023

Choose a reason for hiding this comment

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

Good practice wants that asynchronous code should not block the activation of the plugin as it may delay the full application start up.

A better pattern is therefore to use a Promise approach:

Suggested change
let settings: ISettingRegistry.ISettings;
try {
settings = await settingRegistry.load(variableinspector.id);
} catch (error) {
console.error('Failed to load settings for the Git Extension', error);
}
Promise.all([settingRegistry.load(variableinspector.id), app.restored])
.then(settings => {
// ... here use the settings
})
.catch(error => {
console.error('Failed to load settings for the Git Extension', error);
});


/**
* Create and track a new inspector.
*/
function newPanel(): VariableInspectorPanel {
const panel = new VariableInspectorPanel();
const panel = new VariableInspectorPanel(settings);
Copy link
Member

@fcollonval fcollonval Dec 7, 2023

Choose a reason for hiding this comment

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

This is not the best approach as it add an opinionated interface on how the filter state is stored. To ease code maintenance, extensibility and testing, the best is to not passed the settings object but rather to add filter setters on the panel.

Now the trouble raises from how to pass information from the settings only available within the promise to the panel.

The best solution would be to refactor substantially the code as it is a bad idea to embed the view panel in the manager.

But let's go for a simpler fix for now. You could use a PromiseDelegate like this:

const settings = new PromiseDelegate<ISettingRegistry.ISettings>();

Promise.all([settingRegistry.load(variableinspector.id), app.restored])
.then(settings_ => {
  settings.resolve(settings_);
})
.catch(error => {
  console.error('Failed to load settings for the Git Extension', error);
  settings.reject(error);
});

Then change the command execute to be async:

      execute: async () => {
        if (!manager.panel || manager.panel.isDisposed) {
          await settings; // Wait for the settings to be loaded
          manager.panel = newPanel();
          // Best would be to use a single array of filter object: {filter: '', type: ''}[]
          // to clarify the binding between the type and the filter
          manager.panel.filters = settings.filters;
        }
        if (!manager.panel.isAttached) {
          labShell.add(manager.panel, 'main');
        }
        if (manager.source) {
          manager.source.performInspection();
        }
        labShell.activateById(manager.panel.id);
      }


panel.id = 'jp-variableinspector';
panel.title.label = 'Variable Inspector';
Expand Down
70 changes: 53 additions & 17 deletions src/variableinspector.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ import { IVariableInspector } from './tokens';

import wildcardMatch from 'wildcard-match';

import { ISettingRegistry } from '@jupyterlab/settingregistry';

const TITLE_CLASS = 'jp-VarInspector-title';
const PANEL_CLASS = 'jp-VarInspector';
const TABLE_CLASS = 'jp-VarInspector-table';
Expand All @@ -34,13 +36,15 @@ export class VariableInspectorPanel
implements IVariableInspector
{
private _source: IVariableInspector.IInspectable | null = null;
private _settings: ISettingRegistry.ISettings;
private _filteredTable: HTMLDivElement;
private _table: HTMLTableElement;
private _title: HTMLElement;
private _filtered: { type: Array<string>; name: Array<string> };

constructor() {
constructor(settings: ISettingRegistry.ISettings) {
super();
this._settings = settings;
this.addClass(PANEL_CLASS);
this._title = Private.createTitle();
this._title.className = TITLE_CLASS;
Expand All @@ -52,6 +56,7 @@ export class VariableInspectorPanel
this.node.appendChild(this._table as HTMLElement);
this._filtered = { type: [], name: [] };
this.intializeFilteredTable();
this.initalizeFilterButtons();
}

//Sets up the filter table so when the filter button is pressed, a new filter is created
Expand All @@ -74,6 +79,50 @@ export class VariableInspectorPanel
});
}

// Loads filters from ISettings
protected loadFilters() {
this._filtered.type = this._settings.composite['filteredTypes'] as string[];
this._filtered.name = this._settings.composite['filteredNames'] as string[];
}

// Takes filters and saves them in ISettings
protected setFilters(filterType: FILTER_TYPES | null = null) {
if (filterType === 'type' || !filterType) {
this._settings.set('filteredTypes', this._filtered.type);
}
if (filterType === 'name' || !filterType) {
this._settings.set('filteredNames', this._filtered.name);
}
}

// Loads the filter and creates a new button for each
protected initalizeFilterButtons() {
this.loadFilters();
for (let i = 0; i < this._filtered.type.length; i++) {
this.addFilteredButton(this._filtered.type[i], 'type');
}
for (let i = 0; i < this._filtered.name.length; i++) {
this.addFilteredButton(this._filtered.name[i], 'name');
}
}

// Creates new filtered button and adds it to the filtered variable list
protected addFilteredButton(varName: string, filterType: FILTER_TYPES) {
const filterList = this._filteredTable.querySelector(
'.' + FILTER_LIST_CLASS
) as HTMLUListElement;
const newFilteredButton = Private.createFilteredButton(varName, filterType);
newFilteredButton.addEventListener('click', () => {
const filterText = newFilteredButton.querySelector(
'.filtered-variable-button-text'
) as HTMLDivElement;
this.onFilterChange(filterType, filterText.innerHTML, false);
this.addFilteredOutRows();
newFilteredButton.remove();
});
filterList.appendChild(newFilteredButton);
}

// Checks if string is in the filtered array
protected stringInFilter(string: string, filterType: FILTER_TYPES) {
// console.log(this._filtered[filterType]);
Expand Down Expand Up @@ -106,27 +155,14 @@ export class VariableInspectorPanel
return;
}
this._filtered[filterType].push(varName);
const filterList = this._filteredTable.querySelector(
'.' + FILTER_LIST_CLASS
) as HTMLUListElement;
const newFilteredButton = Private.createFilteredButton(
varName,
filterType
);
newFilteredButton.addEventListener('click', () => {
const filterText = newFilteredButton.querySelector(
'.filtered-variable-button-text'
) as HTMLDivElement;
this.onFilterChange(filterType, filterText.innerHTML, false);
this.addFilteredOutRows();
newFilteredButton.remove();
});
filterList.appendChild(newFilteredButton);
this.setFilters(filterType);
this.addFilteredButton(varName, filterType);
this.filterOutTable();
} else {
this._filtered[filterType] = this._filtered[filterType].filter(
filter => filter !== varName
);
this.setFilters(filterType);
}
}

Expand Down
42 changes: 42 additions & 0 deletions yarn.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -867,6 +867,15 @@ __metadata:
languageName: node
linkType: hard

"@jupyterlab/nbformat@npm:^4.0.9":
version: 4.0.9
resolution: "@jupyterlab/nbformat@npm:4.0.9"
dependencies:
"@lumino/coreutils": ^2.1.2
checksum: 9fb2f2e03c749c46dc2ff4a815ba7a7525dae5d0c44b3d9887a6405b869329d9b3db72f69eada145543a8b37172f5466abf3a621f458793b0565d244218d32e2
languageName: node
linkType: hard

"@jupyterlab/notebook@npm:^4.0.0":
version: 4.0.8
resolution: "@jupyterlab/notebook@npm:4.0.8"
Expand Down Expand Up @@ -1006,6 +1015,25 @@ __metadata:
languageName: node
linkType: hard

"@jupyterlab/settingregistry@npm:^4.0.9":
version: 4.0.9
resolution: "@jupyterlab/settingregistry@npm:4.0.9"
dependencies:
"@jupyterlab/nbformat": ^4.0.9
"@jupyterlab/statedb": ^4.0.9
"@lumino/commands": ^2.1.3
"@lumino/coreutils": ^2.1.2
"@lumino/disposable": ^2.1.2
"@lumino/signaling": ^2.1.2
"@rjsf/utils": ^5.1.0
ajv: ^8.12.0
json5: ^2.2.3
peerDependencies:
react: ">=16"
checksum: 7d4c6f3e69ac1e66b7e7c5e53ccfb98a7e073a5a69837b814f368de247ba22f830ac567a6bb231577f6e256b2b2d9c180d50542f43891640e9a5294cb3e7a189
languageName: node
linkType: hard

"@jupyterlab/statedb@npm:^4.0.8":
version: 4.0.8
resolution: "@jupyterlab/statedb@npm:4.0.8"
Expand All @@ -1019,6 +1047,19 @@ __metadata:
languageName: node
linkType: hard

"@jupyterlab/statedb@npm:^4.0.9":
version: 4.0.9
resolution: "@jupyterlab/statedb@npm:4.0.9"
dependencies:
"@lumino/commands": ^2.1.3
"@lumino/coreutils": ^2.1.2
"@lumino/disposable": ^2.1.2
"@lumino/properties": ^2.0.1
"@lumino/signaling": ^2.1.2
checksum: 0a813068476a1e2dad5aebbbe2a339e8931ba4e29c873d59a2baeed05ab71307e5a629802fddeaec666cec14e4bee45e0d733abe0b1ea0dbf930c8a427188e7b
languageName: node
linkType: hard

"@jupyterlab/statusbar@npm:^4.0.8":
version: 4.0.8
resolution: "@jupyterlab/statusbar@npm:4.0.8"
Expand Down Expand Up @@ -1111,6 +1152,7 @@ __metadata:
"@jupyterlab/outputarea": ^4.0.0
"@jupyterlab/rendermime": ^4.0.0
"@jupyterlab/services": ^7.0.0
"@jupyterlab/settingregistry": ^4.0.9
"@jupyterlab/ui-components": ^4.0.0
"@lumino/coreutils": ^2.0.0
"@lumino/datagrid": ^2.0.0
Expand Down