Carry a patch for the engine dropping depth on a config update
ViewConfig::apply_update in perspective-client applies ten fields and
neither group_by_depth nor split_by_depth is among them. A depth therefore
works when a view is created -- table.view({ group_by_depth: 1 }), which is
what the fork's own depth_test.mjs exercises -- and is silently discarded by
restore(), which is how a viewer changes its own configuration. That is why
the EXPAND buttons did nothing however the value was sent.
group_by_depth and the omission are both upstream; the fork mirrored
split_by_depth alongside it faithfully, including the omission. So the column
axis expand/collapse the fork added has the same hole, and pf_app only avoids
it by collapsing columns through a truncated split_by instead.
The patch adds an Option-aware sibling to _apply and applies both fields. It
cannot be built here -- protoc is absent, so the generated protobuf modules
are missing and the crate does not compile for unrelated reasons -- but
cargo check reports nothing against view_config.rs. It sits in ui/vendor with
the rebuild instructions, beside the tarballs it is not yet in.
Meanwhile applyDepth reads the config back after restoring and falls back to
view.set_depth() when the value did not stick, so the buttons work on the
current build. The fallback loses depth on a view rebuild, as it always did;
what is not back is the shim and listeners that used to chase that. The check
clears itself once the engine honours the field.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
744342d519
commit
f5d0f6b2f6
@ -844,6 +844,26 @@ export default function Forecast({ sources = [], sourceId, versions = [], versio
|
||||
// name in a restored config fails the lookup.
|
||||
const { table: _t, ...cfg } = await viewer.save()
|
||||
await viewer.restore({ ...cfg, group_by_depth: d + 1 })
|
||||
|
||||
// Until the engine carries the patch in ui/vendor, restore() drops the depth:
|
||||
// ViewConfig::apply_update applies ten fields and neither depth is among
|
||||
// them, so the value arrives, deserializes, and is discarded before the
|
||||
// engine sees it. Read it back to find out which build we are on, and fall
|
||||
// back to the imperative call when it did not stick.
|
||||
//
|
||||
// The fallback is the old behaviour, warts and all: set_depth lives on the
|
||||
// view, so it is lost whenever the viewer rebuilds one. What is deliberately
|
||||
// not back is the machinery that used to chase that -- no observer shim, no
|
||||
// focus listeners, no retries. Depth is simply re-applied when you next press
|
||||
// a button, which is predictable, and it becomes durable for free once the
|
||||
// engine is rebuilt.
|
||||
const after = await viewer.save()
|
||||
if (after.group_by_depth !== d + 1) {
|
||||
const view = await viewer.getView()
|
||||
await view.set_depth(d)
|
||||
const plugin = await viewer.getPlugin()
|
||||
await plugin.draw(view)
|
||||
}
|
||||
setExpandDepth(d)
|
||||
}
|
||||
|
||||
|
||||
45
ui/vendor/0001-apply-depth-fields-on-config-update.patch
vendored
Normal file
45
ui/vendor/0001-apply-depth-fields-on-config-update.patch
vendored
Normal file
@ -0,0 +1,45 @@
|
||||
diff --git a/rust/perspective-client/src/rust/config/view_config.rs b/rust/perspective-client/src/rust/config/view_config.rs
|
||||
index fa4f36e..360b816 100644
|
||||
--- a/rust/perspective-client/src/rust/config/view_config.rs
|
||||
+++ b/rust/perspective-client/src/rust/config/view_config.rs
|
||||
@@ -526,6 +526,23 @@ impl ViewConfig {
|
||||
}
|
||||
}
|
||||
|
||||
+ /// `_apply` for a field which is itself `Option`, where `None` in the update
|
||||
+ /// means "not mentioned" rather than "clear it". `Option<Option<T>>` would be
|
||||
+ /// needed to express both, and the wire format cannot carry the difference:
|
||||
+ /// these fields are `skip_serializing_if = "Option::is_none"`, so an absent
|
||||
+ /// field and an explicit null arrive identically. To lift a depth, send the
|
||||
+ /// number of levels on that axis rather than clearing it.
|
||||
+ fn _apply_optional<T: PartialEq>(field: &mut Option<T>, update: Option<T>) -> bool {
|
||||
+ match update {
|
||||
+ None => false,
|
||||
+ Some(_) if *field == update => false,
|
||||
+ Some(_) => {
|
||||
+ *field = update;
|
||||
+ true
|
||||
+ },
|
||||
+ }
|
||||
+ }
|
||||
+
|
||||
pub fn reset(&mut self, reset_expressions: bool) {
|
||||
let mut config = Self::default();
|
||||
if !reset_expressions {
|
||||
@@ -568,6 +585,16 @@ impl ViewConfig {
|
||||
changed = Self::_apply(&mut self.windows, update.windows) || changed;
|
||||
changed = Self::_apply(&mut self.group_rollup_mode, update.group_rollup_mode) || changed;
|
||||
changed = Self::_apply(&mut self.split_rollup_mode, update.split_rollup_mode) || changed;
|
||||
+
|
||||
+ // Without these two, a depth can be set when a view is created --
|
||||
+ // `table.view({ group_by_depth })` -- but never through `restore()`, which
|
||||
+ // merges a `ViewConfigUpdate` onto the live config. The field arrives,
|
||||
+ // deserializes, and is then dropped here, so the viewer's config and the
|
||||
+ // engine never see it and nothing happens. That makes expand/collapse
|
||||
+ // unreachable for anything driven by `restore()`, which is how a viewer
|
||||
+ // changes its own configuration.
|
||||
+ changed = Self::_apply_optional(&mut self.group_by_depth, update.group_by_depth) || changed;
|
||||
+ changed = Self::_apply_optional(&mut self.split_by_depth, update.split_by_depth) || changed;
|
||||
if self.group_rollup_mode == GroupRollupMode::Total && !self.group_by.is_empty() {
|
||||
tracing::info!("`total` incompatible with `group_by`");
|
||||
changed = true;
|
||||
46
ui/vendor/README.md
vendored
46
ui/vendor/README.md
vendored
@ -48,3 +48,49 @@ This is a fork, with the maintenance that implies. The exit is upstream taking
|
||||
the change — the patch is small and additive, and the engine work is already
|
||||
theirs. When a release ships it, delete this directory and put normal version
|
||||
ranges back in `ui/package.json`.
|
||||
|
||||
## Pending patch: depth fields on a config update
|
||||
|
||||
`0001-apply-depth-fields-on-config-update.patch` is not in the vendored
|
||||
tarballs yet. It fixes an upstream bug in
|
||||
`rust/perspective-client/src/rust/config/view_config.rs`:
|
||||
`ViewConfig::apply_update` applies ten fields and **neither `group_by_depth`
|
||||
nor `split_by_depth` is among them**. So a depth can be set when a view is
|
||||
created (`table.view({ group_by_depth: 1 })` — which is what the fork's own
|
||||
`depth_test.mjs` exercises) but never through `restore()`, which is how a
|
||||
viewer changes its own configuration. The field arrives, deserializes, and is
|
||||
discarded before the engine sees it.
|
||||
|
||||
`group_by_depth` and the omission are both upstream; the fork mirrored
|
||||
`split_by_depth` alongside it faithfully, including the omission.
|
||||
|
||||
Note the semantics, from `server.cpp`:
|
||||
|
||||
```cpp
|
||||
ctx1->set_depth(row_pivot_depth - 1); // one-sided
|
||||
ctx2->set_depth(t_header::HEADER_ROW, row_pivot_depth - 1); // two-sided
|
||||
```
|
||||
|
||||
The config field counts **levels to show**; `view.set_depth()` counts the
|
||||
boundary below them. So `group_by_depth: n` equals `set_depth(n - 1)`, and
|
||||
`Forecast.jsx` sends `d + 1`.
|
||||
|
||||
To apply:
|
||||
|
||||
```bash
|
||||
cd $PSP_DIR # default ~/perspective
|
||||
git apply /path/to/pf_app/ui/vendor/0001-apply-depth-fields-on-config-update.patch
|
||||
./ui/vendor/rebuild-perspective.sh
|
||||
cd ui && npm install
|
||||
```
|
||||
|
||||
`cargo check` on this patch was clean — the 125 errors it reports without
|
||||
`protoc` installed are unresolved generated protobuf modules, none of them in
|
||||
`view_config.rs`.
|
||||
|
||||
Until the rebuild lands, `applyDepth()` in `Forecast.jsx` reads the config back
|
||||
after restoring it and falls back to the imperative `view.set_depth()` when the
|
||||
value did not stick. That fallback loses the depth whenever the viewer rebuilds
|
||||
its view; the declarative path does not, which is the point of the patch. The
|
||||
check is cheap and self-clearing — once the engine honours the field, the
|
||||
fallback stops running on its own.
|
||||
|
||||
Loading…
Reference in New Issue
Block a user