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.

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
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
7 changes: 5 additions & 2 deletions package.json
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 @@ -61,6 +62,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 @@ -96,7 +98,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
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": []
}
}
}
25 changes: 20 additions & 5 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,10 @@ import { VariableInspectorManager } from './manager';
import { VariableInspectorPanel } from './variableinspector';

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

import { addJupyterLabThemeChangeListener } from '@jupyter/web-components';
import { PromiseDelegate } from '@lumino/coreutils';
addJupyterLabThemeChangeListener();
namespace CommandIDs {
export const open = 'variableinspector:open';
Expand All @@ -34,28 +37,40 @@ 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 });
const settings = new PromiseDelegate<ISettingRegistry.ISettings>();

Promise.all([settingRegistry.load(variableinspector.id), app.restored])
.then(returnArray => {
return settings.resolve(returnArray[0]);
Comment on lines +60 to +61
Copy link
Member

Choose a reason for hiding this comment

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

To make the code more readable, you could use arg expansion:

Suggested change
.then(returnArray => {
return settings.resolve(returnArray[0]);
.then(([settings_]) => {
return settings.resolve(settings_);

})
.catch(error => {
console.error('Failed to load settings for the Git Extension', error);
settings.reject(error);
});

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

panel.id = 'jp-variableinspector';
panel.title.label = 'Variable Inspector';
Expand Down
88 changes: 72 additions & 16 deletions src/variableinspector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ provideJupyterDesignSystem().register(

import wildcardMatch from 'wildcard-match';

import { ISettingRegistry } from '@jupyterlab/settingregistry';
import { PromiseDelegate } from '@lumino/coreutils';

const TITLE_CLASS = 'jp-VarInspector-title';
const PANEL_CLASS = 'jp-VarInspector';
const TABLE_CLASS = 'jp-VarInspector-table';
Expand All @@ -60,6 +63,10 @@ export class VariableInspectorPanel
implements IVariableInspector
{
private _source: IVariableInspector.IInspectable | null = null;
private _settingsPromise:
| PromiseDelegate<ISettingRegistry.ISettings>
| undefined;
private _settings: ISettingRegistry.ISettings | undefined;
private _table: WebDataGrid;
private _filteredTable: HTMLDivElement;
private _title: HTMLElement;
Expand All @@ -80,6 +87,16 @@ export class VariableInspectorPanel
this.intializeFilteredTable();
}

set settingsPromise(
settingPromise: PromiseDelegate<ISettingRegistry.ISettings>
) {
this._settingsPromise = settingPromise;
this._settingsPromise.promise.then(settings => {
this._settings = settings;
this.initalizeFilterButtons();
});
}

Comment on lines +90 to +99
Copy link
Member

Choose a reason for hiding this comment

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

This is not a great pattern as we should avoid making this class aware of the settings registry.

To do this, this class need to have public setter for the filters and a signal that is emitted when they change. This signal will be used to store new filters to the settings.

The pseudo code would look like that:

// index.ts

panel.filters = settings.composite['filters];
panel.filtersChanged.connect((_, newFilters) => { settings.set('filters', newFilters); });

// variableinspector.ts

class VariableInspectorPanel {
  private _filtersChanged = new Signal<this, string[]>(this);
  private _filters = new Array<string>();

  set filters(value: string[]) {
    this._filters = value;
    this._filtersChanged.emit(this._filters);
  }

  get filterChanged(): ISignal<this, string[]> {
    return this._filtersChanged;
  }
}

}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fcollonval Sorry for the delay. I was just wondering how we are able to access the settings in the index for the two lines.

panel.filters = settings.composite['filters];
panel.filtersChanged.connect((_, newFilters) => { settings.set('filters', newFilters); });

Currently, the settings are a promise in which we load the settings.

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

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

However, when we try to access the settings with the filter, the promise has not resolved yet. How do we go about this?

Copy link
Member

@fcollonval fcollonval Jan 17, 2024

Choose a reason for hiding this comment

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

Hey @kentarolim10 - nice to read you.


A common pattern is to call a onSettingsChanged method in the plugin activation function that propagate the settings to the appropriate objects when they changed; see e.g. https://github.com/jupyterlab-contrib/search-replace/blob/8f4bdff704339cb189e4d54beb230a1e510000f7/src/index.ts#L75

In this case to set the settings on all variable-inspector panels, you could make use of the tracker.forEach method.

//Sets up the filter table so when the filter button is pressed, a new filter is created
protected intializeFilteredTable() {
const filterType = this._filteredTable.querySelector(
Expand All @@ -100,6 +117,58 @@ export class VariableInspectorPanel
});
}

// Loads filters from ISettings
protected loadFilters() {
if (this._settings) {
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 (this._settings) {
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 @@ -132,27 +201,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
10 changes: 10 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,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.9
resolution: "@jupyterlab/notebook@npm:4.0.9"
Expand Down Expand Up @@ -1125,6 +1134,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