Relocate the log hook #433

Currently, composability of X actions is broken, or at least flawed. The culprits are `windows` and a misplacement of responsibility. Every operation that needs to make changes to visible parts of the windowset must run them through the core model-rendering function, yet when time comes to compose those operations, there's no way to prevent intermediate states from being rendered. All of these additional renders are potentially expensive or otherwise deleterious to user experience; they can map and unmap windows, resize them multiple times, wake sleeping or swapped processes, etc. This can easily result in major slowdowns and visible artefacts such as windows jumping around, workspaces flashing past or borders flickering. The original design mitigated these issues by having `windows` accept composable windowset-modifying functions, however, this approach only works within the scope of a given action; it doesn't help to fuse unrelated actions. Further, interleaving any kind of layout operation or IO fundamentally breaks the paradigm. Some efforts have been made to remedy the problem at the contrib level; `X.A.MessageFeedback` and `X.U.PureX` in particular. However, they cannot be considered successful: they require actions to be rewritten in their paradigm, and must foist the task of transforming composable actions into "complete" ones onto the end user. This is not the righteous way. If we instead recognise that rendering the view is the responsibility of the core, a real solution becomes clear: we must take `windows` back from both the hands of the end user and the contrib module implementor. The way to do so transparently is to replace `windows` with a function that, rather than /performing/ a refresh, merely /requests/ one. In the paradigm of `X.U.PureX`, this is achieved by combining actions that produce `Any`, however, this approach does not suffice for our end: it breaks code by changing types, it forces changes in implementation to manually combine the `Any`s, and it interferes with actual return values. The proper tool to issue and combine monoidal values in a monadic context is the writer monad, hence we extend the X monad to write `Any`.

`refresh` becomes our deviation-declaring, refresh-requesting operation, and its pair `norefresh` is introduced to deny such requests or mark them handled. As promised, we now make `windows` internal by renaming it `render`, providing a replacement that just modifies the windowset and issues `refresh`. `windowBracket` is cannibalised to become `handleRefresh`, catching requests and handling them through `render`. As with `windows`, we provide a replacement with the corresponding semantics under the new ways. Finally, we run the event handler and startup operations inside `handleRefresh`, completing the change of regime. /Note that this change will break any functionality actually requiring `windows` to perform an immediate refresh./ The solution would be to wrap any such use of `windows` in `handleRefresh`, but it currently offers poor support for nesting. This matter will be rectified in a later commit.

Under the new regime, an operation should issue `refresh` if it may directly cause the model to deviate from the view, and should avoid doing so when it knows it will not. Hence: * `updateLayout` issues `refresh` iff the target workspace is visible. For reusability, this workspace-updating logic is implemented as the more general `respace`. * `modifyWindowSet` is deprecated. * `sendMessageWithNoRefresh` is replaced by `messageWorkspace` and deprecated. * `broadcastMessage` may now (indirectly) issue `refresh`. `windowBracket` and co. were originally introduced so that windowset changes made in `sendMessage` would be properly handled in the accompanying refresh. The approach has since been adopted for use in `handleRefresh`, so these functions are no longer necessary and don't belong in the core. As such, `sendMessage` is simplified and the "bracket" functions are deprecated.

There was unnecessary noise and duplication in these functions. * `sendMessage`: Rewrite via `messageWorkspace` * `setLayout`: Rewrite via `sendMessage` and `updateLayout` * `manage`: Remove code equivalent to `W.view (W.currentTag ws) ws` The `sendMessage` rewrite also fixes a corner case misbehaviour: previously, if message handling resulted in another workspace taking focus, the layout update would mangle that workspace. This is no longer the case.

`render` requires the previously rendered state of the model---the current state of the view---to judge changes against. Originally, this was done by taking the windowset for that state and requiring changes be provided in the form of a function. Under that regime, the windowset is not a fine grained internal model---its merely a log of the view which shouldn't be exposed for editing outside of `render`. `handleRefresh` (previously `windowBracket`) used a cute trick to buy granularity within a child action by grabbing the "old" and the "new" copies of the windowset that `render` needs before and after the action runs. However, such a trick has its limits. In particular, it does not support nesting---a refresh in the child action invalidates the "old" copy. The solution is straightforward: separately from our model, keep an actual log of the state of the view, and don't expose it for editing. Hence it does not appear in the `XState`, but is secreted away in an `IORef` in a new opaque `Internal` portion of the `XConf`.

Using `handleRefresh` as we are, a longstanding issue with the `logHook` is fixed, or rather, transformed: running `windows` within it will not cause an infinite loop; any requests it issues are silently discarded. That, however, is not ideal either: unhandled changes may be introduced to the windowset. We could run the `logHook` via `handleRefresh`, but that would put us back where we started with infinite loops. Alternatively, we could treat a request in the `logHook` as an error and discard its changes à la `catchX`. However, the way that gives the best bang for buck is to move the `logHook` from `render` into `handleRefresh`, after the user effects have run but before the refresh itself. Like this, any changes it makes are seamlessly handled in the refresh it accompanies. This is simple, efficient, and best of all: it transforms a mere `logHook` into a fully fledged "refreshHook". While the `logHook` is seeing essentially the same state as far as the internal model goes, there's one key observable difference: it runs before the effects in `render`, and thus it does not see any changes that might result from running the layout. This could break some existing usage.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Relocate the log hook #433

Relocate the log hook #433

Commits on Dec 19, 2022

Relocate the log hook #433

Are you sure you want to change the base?

Relocate the log hook #433

Commits on Dec 19, 2022