Skip to content

feat: move display Godley table variable values as an attribute of Go… #662

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

Merged
highperformancecoder merged 5 commits into from
May 29, 2026
Merged

feat: move display Godley table variable values as an attribute of Go… #662

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
6e7cf4d
feat: move display Godley table variable values as an attribute of Go…
highperformancecoder May 29, 2026
552b985
Potential fix for pull request finding
highperformancecoder May 29, 2026
bcb0789
chore: remove useless unit test
highperformancecoder May 29, 2026
2d94995
Merge branch 'ture-194-show-values-as-intrinsic' of github.com:highpe…
highperformancecoder May 29, 2026
cbbd7b7
bigfix: Fix failing regression test
highperformancecoder May 29, 2026
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
3 changes: 1 addition & 2 deletions gui-js/apps/minsky-electron/src/app/managers/CommandsManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1235,12 +1235,11 @@ export class CommandsManager {
static async applyPreferences() {
const {
enableMultipleEquityColumns,
godleyTableShowValues,
godleyTableOutputStyle,
font,
numBackups,
} = StoreManager.store.get('preferences');
minsky.setGodleyDisplayValue(godleyTableShowValues,godleyTableOutputStyle);
minsky.displayStyle(godleyTableOutputStyle);
minsky.multipleEquities(enableMultipleEquityColumns);
minsky.defaultFont(font);
minsky.numBackups(numBackups);
Expand Down
14 changes: 13 additions & 1 deletion gui-js/apps/minsky-electron/src/app/managers/ContextMenuManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -682,7 +682,19 @@ export class ContextMenuManager {
label: 'Display variables',
type: 'checkbox',
checked: displayVariableChecked,
click: () => godley.toggleVariableDisplay()
click: async () => {
await godley.toggleVariableDisplay();
await minsky.canvas.requestRedraw();
}
}),
new MenuItem({
label: 'Display values',
type: 'checkbox',
checked: await godley.displayValues(),
click: async () => {
await godley.toggleDisplayValues();
await minsky.canvas.requestRedraw();
},
Comment on lines +694 to +697
Copy link
Copy Markdown
Owner Author

@highperformancecoder highperformancecoder May 29, 2026

Choose a reason for hiding this comment

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

Popups no longer use this code, but rather HTML table rendering.

Copy link
Copy Markdown
Owner Author

@highperformancecoder highperformancecoder May 29, 2026

Choose a reason for hiding this comment

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

I should remove the no longer used popup member, but that is for a later refactor.

}),
new MenuItem({
label: 'Copy flow variables',
Expand Down
10 changes: 2 additions & 8 deletions gui-js/apps/minsky-electron/src/app/managers/GodleyMenuManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,27 +58,21 @@ export class GodleyMenuManager {
const preferences = StoreManager.store.get('preferences');
let {
enableMultipleEquityColumns,
godleyTableShowValues,
godleyTableOutputStyle,
} = preferences;

if (property === 'enableMultipleEquityColumns') {
enableMultipleEquityColumns = value as boolean;
minsky.multipleEquities(enableMultipleEquityColumns);
GodleyMenuManager.refresh(window);
} else {
if (property === 'godleyTableOutputStyle') {
} else if (property === 'godleyTableOutputStyle') {
godleyTableOutputStyle = value as GodleyTableOutputStyles;
} else if (property === 'godleyTableShowValues') {
godleyTableShowValues = value as boolean;
}
minsky.setGodleyDisplayValue(godleyTableShowValues, godleyTableOutputStyle);
minsky.displayStyle(godleyTableOutputStyle);
}

StoreManager.store.set({
preferences: {
...preferences,
godleyTableShowValues: godleyTableShowValues,
godleyTableOutputStyle: godleyTableOutputStyle,
enableMultipleEquityColumns: enableMultipleEquityColumns,
},
Expand Down
2 changes: 0 additions & 2 deletions gui-js/apps/minsky-electron/src/app/managers/StoreManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import Store from 'electron-store';
import {homedir} from 'node:os';

interface MinskyPreferences {
godleyTableShowValues: boolean;
godleyTableOutputStyle: string;
enableMultipleEquityColumns: boolean;
numberOfRecentFilesToDisplay: number;
Expand Down Expand Up @@ -31,7 +30,6 @@ class StoreManager {
defaultModelDirectory: homedir(),
defaultDataDirectory: homedir(),
preferences: {
godleyTableShowValues: false,
godleyTableOutputStyle: 'sign',
enableMultipleEquityColumns: false,
numberOfRecentFilesToDisplay: 10,
Expand Down
11 changes: 0 additions & 11 deletions gui-js/libs/menu/src/lib/options/preferences/preferences.component.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,6 @@

<div class="form-wrapper">
<form [formGroup]="form">
<div class="form-control">
<label for="godleyTableShowValues">Godley Table Show Values</label>
<input
formControlName="godleyTableShowValues"
type="checkbox"
class="godley-table-show-values-checkbox"
id="godleyTableShowValues"
(change)="updatePreferences()"
/>
</div>

<div class="form-control">
<label>Godley Table Output Style</label>
<span class="radio-wrapper">
Expand Down
1 change: 0 additions & 1 deletion gui-js/libs/menu/src/lib/options/preferences/preferences.component.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ export class PreferencesComponent implements OnInit {

constructor(private electronService: ElectronService) {
this.form = new FormGroup({
godleyTableShowValues: new FormControl(null),
godleyTableOutputStyle: new FormControl(null),
enableMultipleEquityColumns: new FormControl(null),
numberOfRecentFilesToDisplay: new FormControl(null),
Expand Down
4 changes: 2 additions & 2 deletions gui-js/libs/shared/src/lib/backend/minsky.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,7 @@ export class GodleyIcon extends Item {
async currency(...args: string[]): Promise<string> {return this.$callMethod('currency',...args);}
async deleteRow(a1: number): Promise<void> {return this.$callMethod('deleteRow',a1);}
async destroyFrame(): Promise<void> {return this.$callMethod('destroyFrame');}
async displayValues(...args: boolean[]): Promise<boolean> {return this.$callMethod('displayValues',...args);}
async draw(a1: ICairoShim): Promise<void> {return this.$callMethod('draw',a1);}
async editorMode(): Promise<boolean> {return this.$callMethod('editorMode');}
async flowSignature(a1: number): Promise<object> {return this.$callMethod('flowSignature',a1);}
Expand Down Expand Up @@ -771,6 +772,7 @@ export class GodleyIcon extends Item {
async toEditorX(a1: number): Promise<number> {return this.$callMethod('toEditorX',a1);}
async toEditorY(a1: number): Promise<number> {return this.$callMethod('toEditorY',a1);}
async toggleButtons(): Promise<void> {return this.$callMethod('toggleButtons');}
async toggleDisplayValues(): Promise<void> {return this.$callMethod('toggleDisplayValues');}
async toggleEditorMode(): Promise<void> {return this.$callMethod('toggleEditorMode');}
async toggleVariableDisplay(): Promise<void> {return this.$callMethod('toggleVariableDisplay');}
async update(): Promise<void> {return this.$callMethod('update');}
Expand Down Expand Up @@ -1368,7 +1370,6 @@ export class Minsky extends CppClass {
async dimensionalAnalysis(): Promise<void> {return this.$callMethod('dimensionalAnalysis');}
async displayErrorItem(a1: Item): Promise<void> {return this.$callMethod('displayErrorItem',a1);}
async displayStyle(...args: string[]): Promise<string> {return this.$callMethod('displayStyle',...args);}
async displayValues(...args: boolean[]): Promise<boolean> {return this.$callMethod('displayValues',...args);}
async doPushHistory(...args: boolean[]): Promise<boolean> {return this.$callMethod('doPushHistory',...args);}
async ecolabVersion(): Promise<string> {return this.$callMethod('ecolabVersion');}
async edited(): Promise<boolean> {return this.$callMethod('edited');}
Expand Down Expand Up @@ -1454,7 +1455,6 @@ export class Minsky extends CppClass {
async setAutoSaveFile(a1: string): Promise<void> {return this.$callMethod('setAutoSaveFile',a1);}
async setBusyCursor(): Promise<void> {return this.$callMethod('setBusyCursor');}
async setDefinition(a1: string,a2: string): Promise<void> {return this.$callMethod('setDefinition',a1,a2);}
async setGodleyDisplayValue(a1: boolean,a2: string): Promise<void> {return this.$callMethod('setGodleyDisplayValue',a1,a2);}
async setGodleyIconResource(a1: string): Promise<void> {return this.$callMethod('setGodleyIconResource',a1);}
async setGroupIconResource(a1: string): Promise<void> {return this.$callMethod('setGroupIconResource',a1);}
async setLockIconResource(a1: string,a2: string): Promise<void> {return this.$callMethod('setLockIconResource',a1,a2);}
Expand Down
7 changes: 6 additions & 1 deletion model/godleyIcon.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,13 @@ namespace minsky
GodleyTableEditor editor{*this};
/// for rendering the popup window
GodleyTableWindow popup{*this};
void adjustPopupWidgets() {popup.adjustWidgets();}

/// whether to display values in this table
bool displayValues=false;
void toggleDisplayValues() {displayValues=!displayValues;}
Comment on lines +86 to +88
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 29, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Redraw the popup when this flag changes.

toggleDisplayValues() only flips the bit. If this Godley table is already open in its popup window, it will keep showing the old rendering until some unrelated event triggers popup.requestRedraw(). The previous global path handled that redraw, so this refactor regresses live updates for open tables unless the per-icon toggle path does it too.

Suggested fix 
-    void toggleDisplayValues() {displayValues=!displayValues;}
+    void toggleDisplayValues()
+    {
+      displayValues=!displayValues;
+      popup.requestRedraw();
+    }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
/// whether to display values in this table
bool displayValues=false;
void toggleDisplayValues() {displayValues=!displayValues;}
/// whether to display values in this table
bool displayValues=false;
void toggleDisplayValues()
{
displayValues=!displayValues;
popup.requestRedraw();
}
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@model/godleyIcon.h` around lines 86 - 88, toggleDisplayValues currently only
flips the displayValues flag and doesn't trigger a visual update; update
toggleDisplayValues() in GodleyIcon to call the popup redraw path after flipping
the flag (e.g., if the associated popup/window for this Godley table exists/is
open, call popup.requestRedraw() or the equivalent method used elsewhere to
force a repaint) so open popups immediately reflect the change.

Copy link
Copy Markdown
Owner Author

@highperformancecoder highperformancecoder May 29, 2026

Choose a reason for hiding this comment

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

Popups no longer use this code. I'll add a ticket to refactor this member away.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 29, 2026

Choose a reason for hiding this comment

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

@highperformancecoder, thanks for the clarification — that makes sense. Would you like me to open a GitHub issue to track the refactoring of the popup member away from this code path?


void adjustPopupWidgets() {popup.adjustWidgets();}

/// scale icon until it's height or width matches \a h or \a w depending on which is minimum
void scaleIcon(float w, float h);

Expand Down
4 changes: 2 additions & 2 deletions model/godleyTableWindow.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ namespace minsky
{
string value;
FlowCoef fc(text);
if (cminsky().displayValues && col!=0) // Do not add value "= 0.0" to first column. For tickets 1064/1274
if (m_godleyIcon.displayValues && col!=0) // Do not add value "= 0.0" to first column. For tickets 1064/1274
try
{
auto vv=cminsky().variableValues
Expand Down Expand Up @@ -280,7 +280,7 @@ namespace minsky
}
else
//Display values of parameters used as initial conditions in Godley tables. for ticket 1126.
if (m_godleyIcon.table.initialConditionRow(row) && cminsky().displayValues) text=defang(text+=value);
if (m_godleyIcon.table.initialConditionRow(row) && m_godleyIcon.displayValues) text=defang(text+=value);
else text=defang(text);
}
pango.setMarkup(text);
Expand Down
3 changes: 3 additions & 0 deletions model/godleyTableWindow.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,9 @@ namespace minsky
/// minimum column width (for eg empty columns)
static constexpr double minColumnWidth=4*ButtonWidget<col>::buttonSpacing;

// note explicit getter/setter here rather than public member to
// avoid an infinite recursion when running RESTProcess on this
// object
GodleyIcon& godleyIcon() {return m_godleyIcon;}
const GodleyIcon& godleyIcon() const {return m_godleyIcon;}

Expand Down
12 changes: 0 additions & 12 deletions model/minsky.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1597,18 +1597,6 @@ namespace minsky
});
}

void Minsky::setGodleyDisplayValue(bool displayValues, GodleyTable::DisplayStyle displayStyle)
{
this->displayValues=displayValues;
this->displayStyle=displayStyle;
canvas.requestRedraw();
model->recursiveDo(&GroupItems::items, [](Items&,Items::iterator i) {
if (auto g=dynamic_cast<GodleyIcon*>(i->get()))
g->popup.requestRedraw();
return false;
});
}

void Minsky::importVensim(const string& filename)
{
ifstream f(filename);
Expand Down
4 changes: 0 additions & 4 deletions model/minsky.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,12 +424,8 @@ namespace minsky
void srand(int seed) {::srand(seed);}

// godley table display values preferences
bool displayValues=false;
GodleyTable::DisplayStyle displayStyle=GodleyTable::sign;

/// set display value mode on all godley table editor modes
void setGodleyDisplayValue(bool displayValues, GodleyTable::DisplayStyle displayStyle);

/// import a Vensim file
void importVensim(const std::string&);

Expand Down
2 changes: 2 additions & 0 deletions schema/schema3.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,8 @@ namespace schema3
SchemaHelper::setVariableDisplay(*x1, *y.variableDisplay);
if (y.buttonDisplay && *y.buttonDisplay!=x1->buttonDisplay())
x1->toggleButtons();
if (y.displayValues && *y.displayValues!=x1->displayValues)
x1->toggleDisplayValues();
if (y.currency) x1->currency=*y.currency;
}
if (auto* x1=dynamic_cast<minsky::PlotWidget*>(&x))
Expand Down
5 changes: 3 additions & 2 deletions schema/schema3.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ namespace schema3
// Godley Icon specific fields
Optional<std::vector<std::vector<std::string>>> data;
Optional<std::vector<minsky::GodleyAssetClass::AssetClass>> assetClasses;
Optional<bool> editorMode, buttonDisplay, variableDisplay;
Optional<bool> editorMode, buttonDisplay, variableDisplay, displayValues;
Optional<string> currency;
// sheet specific fields
Optional<minsky::ShowSlice> showSlice; // slicing rows
Expand Down Expand Up @@ -160,7 +160,8 @@ namespace schema3
ItemBase(id,static_cast<const minsky::Item&>(g),ports),
data(g.table.getData()), assetClasses(g.table.assetClass()),
editorMode(g.editorMode()), buttonDisplay(g.buttonDisplay()),
variableDisplay(g.variableDisplay()), currency(g.currency)
variableDisplay(g.variableDisplay()), displayValues(g.displayValues),
currency(g.currency)
{name=g.table.title;}
Item(int id, const minsky::PlotWidget& p, const std::vector<int>& ports):
ItemBase(id,static_cast<const minsky::Item&>(p),ports),
Expand Down
4 changes: 2 additions & 2 deletions test/00/godleyTableWindow.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ minsky.defaultFont('Sans')
minsky.load('$here/examples/1Free.mky')
minsky.multipleEquities(True)
minsky.displayStyle('sign')
minsky.displayValues(True)
item=findObject('GodleyIcon')
item.displayValues(True)
gw=item.popup
gw.enableButtons()
gw.renderToSVG('1FreeBase.svg')
Expand All @@ -34,7 +34,7 @@ gw.renderToSVG('1FreeSelectedCol.svg')
minsky.load('$here/examples/LoanableFunds.mky')
item=findObject('GodleyIcon')
minsky.displayStyle('DRCR')
minsky.displayValues(0)
item.displayValues(False)
item.popup.enableButtons()
item.popup.renderToSVG('LoanableFundsBase.svg')
EOF
Expand Down
15 changes: 0 additions & 15 deletions test/testMinsky.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1628,21 +1628,6 @@ TEST(TensorOps, evalOpEvaluate)
EXPECT_TRUE(variableValues[":unitVar"]->units.str().empty());
}

// Test setGodleyDisplayValue
TEST_F(MinskySuite, setGodleyDisplayValue)
{
auto g1 = new GodleyIcon;
model->addItem(g1);

setGodleyDisplayValue(true, GodleyTable::DRCR);
EXPECT_TRUE(displayValues);
EXPECT_EQ(GodleyTable::DRCR, displayStyle);

setGodleyDisplayValue(false, GodleyTable::sign);
EXPECT_FALSE(displayValues);
EXPECT_EQ(GodleyTable::sign, displayStyle);
}

// Test save and load
TEST_F(MinskySuite, saveAndLoad)
{
Expand Down
Loading