Quality pass: reuse, dead weight, and two real costs
Four review angles over the Nim port and the renderer. What got fixed: Reuse. `summarise` and `previewLine` were the same whitespace-collapse-then- rune-truncate function written twice, comment and all; one `summarise` lives in `textruns` now, which is the module for string work on message text, and `previewLine` is a call to it. `floorDiv`/`floorMod` were hand-rolled where std/math has exactly those semantics. Every FFI entry point is resolved once into a `final` instead of a `dlsym` plus a fresh trampoline on each call, and the two-argument marshalling is one `_call2` beside `_call1`. Dead weight. Nine unused imports, so the next real warning is visible. An unused `room` parameter. `belowList` — a proc with an unused argument returning the literal 46 — and the `reserve` prop it fed, which the renderer's own comment says it ignores on purpose. A line on the connect screen that had stopped being true: "TLS comes from dart:io" is what it said, on a path where TLS is Nim's std/net. That one is user-visible text, not a comment. And `firstImageUrl` was ported, tested, and never called — `reducer` assigned `imageUrl` its own default instead, so no received message ever showed the inline preview the chat screen draws. One line. Two costs worth the name. `currentRoom` returns a Room **by value**, and a Room owns its whole backlog: resolving a reply called it once per replying line per render, which in a busy room is tens of thousands of message deep-copies a frame, ten times a second. The room is threaded down now. And `localOffsetSeconds` was a `localtime_r` — a stat of /etc/localtime — three times per timestamp per render; it is memoised per UTC day, which is exact rather than approximate since the offset never moves inside a day. The poll loop compared two trees by stringifying both, ten times a second, to answer "did anything change". It compares the JSON Nim already produced and decodes only on a miss. Altitude. `hbox` + `wrap: true` + `inline: true` was three props saying "paragraph" and a renderer branch that shared nothing with either row path and threw away the children it had just built. It is a `paragraph` tag. And `fillHeight`-on-a-vbox, a bare `scroll`, and a row peering at its children's props were three spellings of "take the remaining extent" — one `expand` prop, stated by the node that expands, handled once. One finding I applied and reverted: flipping `hbox`'s wrap default to false, on the grounds that the tree states `wrap` at every call site. It does not — 4 of 15 do — so the other 11 became Rows and overflowed. The layout tests caught it, which is the second time this week they have caught something I was about to be confident about. Skipped, deliberately: the unused `State` fields, the `dialog` tag, the Overview toggle and `edits.nim` are all staging for screens not yet ported — real dead weight today, churn to delete and re-add. `frq_ui_demo` stays in the ABI beside `frq_ui_reset`, which can wipe a session just as thoroughly. And `clock.nim`'s Hinnant arithmetic stays: std/times could do it, but it is tested against a fifty-thousand-day round trip and rewriting working date code for line count is not a quality improvement. 203 Nim tests, 20 Dart, 21 layout, check-common clean, no compiler warnings, and the window still reaches #test with 114 lines and no layout errors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
4dfc719 parent: 32aed8b modified
dart/frq_core/lib/frq_core.dart +93 -38 | @@ -91,7 +91,27 @@ final DynamicLibrary _lib = () { | ||
| 91 | 91 | return lib; |
| 92 | 92 | }(); |
| 93 | 93 | |
| 94 | +// Every entry point resolved once, here, rather than on each call. | |
| 95 | +// | |
| 96 | +// `lookupFunction` is a dlsym plus a freshly built trampoline closure every | |
| 97 | +// time it runs. At 10Hz for `poll` and once per keystroke for `dispatch` that | |
| 98 | +// is measurable and, more to the point, free to avoid — these are `final`, so | |
| 99 | +// they cost one lookup for the life of the process. | |
| 94 | 100 | final _free = _lib.lookupFunction<_FreeNative, _FreeDart>('frq_free'); |
| 101 | +final _version = _lib.lookupFunction<_VersionNative, _VersionDart>('frq_version'); | |
| 102 | +final _tagValue = _lib.lookupFunction<_Str2Native, _Str2Dart>('frq_irc_tag_value'); | |
| 103 | +final _traceFn = _lib.lookupFunction<_Str2Native, _Str2Dart>('frq_trace'); | |
| 104 | +final _connOpen = _lib.lookupFunction<_ConnOpenNative, _ConnOpenDart>('frq_conn_open'); | |
| 105 | +final _connSend = _lib.lookupFunction<_Str1Native, _Str1Dart>('frq_conn_send'); | |
| 106 | +final _connCloseFn = _lib.lookupFunction<_VoidNative, _VoidDart>('frq_conn_close'); | |
| 107 | +final _connRecvFn = _lib.lookupFunction<_Str0Native, _Str0Dart>('frq_conn_recv'); | |
| 108 | +final _connEventFn = _lib.lookupFunction<_Str0Native, _Str0Dart>('frq_conn_event'); | |
| 109 | +final _uiRender = _lib.lookupFunction<_Str0Native, _Str0Dart>('frq_ui_render'); | |
| 110 | +final _uiPoll = _lib.lookupFunction<_Str0Native, _Str0Dart>('frq_ui_poll'); | |
| 111 | +final _uiDispatch = _lib.lookupFunction<_Str1Native, _Str1Dart>('frq_ui_dispatch'); | |
| 112 | +final _uiDemo = _lib.lookupFunction<_VoidNative, _VoidDart>('frq_ui_demo'); | |
| 113 | +final _uiReset = _lib.lookupFunction<_VoidNative, _VoidDart>('frq_ui_reset'); | |
| 114 | +final _str1 = <String, _Str1Dart>{}; | |
| 95 | 115 | |
| 96 | 116 | /// The bytes at [p] as a string, with [p] freed afterwards. Null in, null out. |
| 97 | 117 | String? _takeString(Pointer<Uint8> p) { |
| @@ -133,9 +153,26 @@ final _malloc = _libc | ||
| 133 | 153 | final _freeArg = |
| 134 | 154 | _libc.lookupFunction<Void Function(Pointer<Uint8>), void Function(Pointer<Uint8>)>('free'); |
| 135 | 155 | |
| 156 | +/// Call a two-strings-in, one-string-out entry point. | |
| 157 | +/// | |
| 158 | +/// The mirror of [_call1], and it exists for the same reason: the | |
| 159 | +/// `_toC`/`try`/`finally`/`_freeArg` dance is four lines of ownership | |
| 160 | +/// bookkeeping that no call site should repeat. | |
| 161 | +String? _call2(_Str2Dart f, String x, String y) { | |
| 162 | + final a = _toC(x); | |
| 163 | + final b = _toC(y); | |
| 164 | + try { | |
| 165 | + return _takeString(f(a, b)); | |
| 166 | + } finally { | |
| 167 | + _freeArg(a); | |
| 168 | + _freeArg(b); | |
| 169 | + } | |
| 170 | +} | |
| 171 | + | |
| 136 | 172 | /// Call a one-string-in, one-string-out entry point. |
| 137 | 173 | String? _call1(String symbol, String arg) { |
| 138 | - final f = _lib.lookupFunction<_Str1Native, _Str1Dart>(symbol); | |
| 174 | + final f = _str1.putIfAbsent( | |
| 175 | + symbol, () => _lib.lookupFunction<_Str1Native, _Str1Dart>(symbol)); | |
| 139 | 176 | final a = _toC(arg); |
| 140 | 177 | try { |
| 141 | 178 | return _takeString(f(a)); |
| @@ -150,7 +187,7 @@ String? _call1(String symbol, String arg) { | ||
| 150 | 187 | /// is the one it was built against. Static storage on the Nim side: the one |
| 151 | 188 | /// return value that is NOT freed. |
| 152 | 189 | String get version { |
| 153 | - final p = _lib.lookupFunction<_VersionNative, _VersionDart>('frq_version')(); | |
| 190 | + final p = _version(); | |
| 154 | 191 | var len = 0; |
| 155 | 192 | while (p[len] != 0) { |
| 156 | 193 | len++; |
| @@ -172,15 +209,7 @@ Map<String, dynamic> parseLine(String line) { | ||
| 172 | 209 | /// One IRCv3 tag's value, unescaped — null where the tag is absent OR empty, |
| 173 | 210 | /// which IRCv3 says are the same thing. |
| 174 | 211 | String? tagValue(String tags, String key) { |
| 175 | - final f = _lib.lookupFunction<_Str2Native, _Str2Dart>('frq_irc_tag_value'); | |
| 176 | - final a = _toC(tags); | |
| 177 | - final b = _toC(key); | |
| 178 | - try { | |
| 179 | - return _takeString(f(a, b)); | |
| 180 | - } finally { | |
| 181 | - _freeArg(a); | |
| 182 | - _freeArg(b); | |
| 183 | - } | |
| 212 | + return _call2(_tagValue, tags, key); | |
| 184 | 213 | } |
| 185 | 214 | |
| 186 | 215 | String unescapeTag(String v) => _call1('frq_irc_unescape_tag', v) ?? ''; |
| @@ -194,15 +223,7 @@ String nickOf(String prefix) => _call1('frq_irc_nick_of', prefix) ?? ''; | ||
| 194 | 223 | /// Log through the Nim core's trace facility, so `FRQ_TRACE=1` gives one |
| 195 | 224 | /// interleaved story rather than two half-ones in different places. |
| 196 | 225 | void trace(String topic, String msg) { |
| 197 | - final f = _lib.lookupFunction<_Str2Native, _Str2Dart>('frq_trace'); | |
| 198 | - final a = _toC(topic); | |
| 199 | - final b = _toC(msg); | |
| 200 | - try { | |
| 201 | - f(a, b); | |
| 202 | - } finally { | |
| 203 | - _freeArg(a); | |
| 204 | - _freeArg(b); | |
| 205 | - } | |
| 226 | + _call2(_traceFn, topic, msg); | |
| 206 | 227 | } |
| 207 | 228 | |
| 208 | 229 | // --------------------------------------------------------------- transport |
| @@ -217,7 +238,7 @@ typedef _ConnOpenDart = void Function(Pointer<Uint8>, int, int); | ||
| 217 | 238 | /// Dial. Non-blocking: the socket runs on a Nim thread and progress arrives |
| 218 | 239 | /// through [connEvent]. |
| 219 | 240 | void connOpen(String host, int port, {bool tls = true}) { |
| 220 | - final f = _lib.lookupFunction<_ConnOpenNative, _ConnOpenDart>('frq_conn_open'); | |
| 241 | + final f = _connOpen; | |
| 221 | 242 | final a = _toC(host); |
| 222 | 243 | try { |
| 223 | 244 | f(a, port, tls ? 1 : 0); |
| @@ -228,7 +249,7 @@ void connOpen(String host, int port, {bool tls = true}) { | ||
| 228 | 249 | |
| 229 | 250 | /// Queue a line. The transport adds the CRLF. |
| 230 | 251 | void connSend(String line) { |
| 231 | - final f = _lib.lookupFunction<_Str1Native, _Str1Dart>('frq_conn_send'); | |
| 252 | + final f = _connSend; | |
| 232 | 253 | final a = _toC(line); |
| 233 | 254 | try { |
| 234 | 255 | f(a); |
| @@ -237,16 +258,13 @@ void connSend(String line) { | ||
| 237 | 258 | } |
| 238 | 259 | } |
| 239 | 260 | |
| 240 | -void connClose() => | |
| 241 | - _lib.lookupFunction<_VoidNative, _VoidDart>('frq_conn_close')(); | |
| 261 | +void connClose() => _connCloseFn(); | |
| 242 | 262 | |
| 243 | 263 | /// The next line, or null when none is waiting. Never blocks. |
| 244 | -String? connRecv() => _takeString( | |
| 245 | - _lib.lookupFunction<_Str0Native, _Str0Dart>('frq_conn_recv')()); | |
| 264 | +String? connRecv() => _takeString(_connRecvFn()); | |
| 246 | 265 | |
| 247 | 266 | /// The next transport event — `open`, `close: …`, `error: …` — or null. |
| 248 | -String? connEvent() => _takeString( | |
| 249 | - _lib.lookupFunction<_Str0Native, _Str0Dart>('frq_conn_event')()); | |
| 267 | +String? connEvent() => _takeString(_connEventFn()); | |
| 250 | 268 | |
| 251 | 269 | |
| 252 | 270 | // ---------------------------------------------------------------- the UI |
| @@ -293,7 +311,23 @@ class UiNode { | ||
| 293 | 311 | } |
| 294 | 312 | |
| 295 | 313 | UiNode _treeFrom(String? json) => |
| 296 | - UiNode.fromJson(jsonDecode(json ?? '{"tag":"vbox"}') as Map<String, dynamic>); | |
| 314 | + UiNode.fromJson(jsonDecode(json ?? _emptyTree) as Map<String, dynamic>); | |
| 315 | + | |
| 316 | +/// A tree and the JSON it came from. | |
| 317 | +/// | |
| 318 | +/// The raw string is kept because it is the cheapest possible change | |
| 319 | +/// detector: Nim already produced it, and comparing two strings is free | |
| 320 | +/// beside decoding one. The renderer polls ten times a second and the answer | |
| 321 | +/// is almost always "nothing changed" — doing a `jsonDecode` and two | |
| 322 | +/// recursive `toString()`s to discover that was most of the idle cost of the | |
| 323 | +/// app in a busy room. | |
| 324 | +const _emptyTree = '{"tag":"vbox"}'; | |
| 325 | + | |
| 326 | +class UiFrame { | |
| 327 | + final String json; | |
| 328 | + final UiNode tree; | |
| 329 | + const UiFrame(this.json, this.tree); | |
| 330 | +} | |
| 297 | 331 | |
| 298 | 332 | /// The current screen. |
| 299 | 333 | /// |
| @@ -301,23 +335,46 @@ UiNode _treeFrom(String? json) => | ||
| 301 | 335 | /// no [dispatch] between can differ when a line arrived in the gap. That is how |
| 302 | 336 | /// the room fills, and why the renderer polls. |
| 303 | 337 | UiNode render() => _treeFrom( |
| 304 | - _takeString(_lib.lookupFunction<_Str0Native, _Str0Dart>('frq_ui_render')())); | |
| 338 | + _takeString(_uiRender())); | |
| 305 | 339 | |
| 306 | 340 | /// The tree, asked for because time passed rather than because anything |
| 307 | 341 | /// happened. Same work as [render]; named for what the caller means. |
| 308 | 342 | UiNode poll() => _treeFrom( |
| 309 | - _takeString(_lib.lookupFunction<_Str0Native, _Str0Dart>('frq_ui_poll')())); | |
| 343 | + _takeString(_uiPoll())); | |
| 344 | + | |
| 345 | +/// The current screen, with the JSON it came from. The starting point for | |
| 346 | +/// [pollIfChanged]. | |
| 347 | +UiFrame renderFrame() { | |
| 348 | + final json = _takeString(_uiRender()) ?? _emptyTree; | |
| 349 | + return UiFrame(json, _treeFrom(json)); | |
| 350 | +} | |
| 351 | + | |
| 352 | +/// The tree, decoded only when it differs from [since] — otherwise null, | |
| 353 | +/// meaning "the screen you already have is current". | |
| 354 | +/// | |
| 355 | +/// This is what the renderer polls with. The comparison is the JSON Nim | |
| 356 | +/// already produced, so an unchanged frame costs one string compare rather | |
| 357 | +/// than a decode and two recursive `toString()`s. | |
| 358 | +UiFrame? pollIfChanged(String since) { | |
| 359 | + final json = _takeString(_uiPoll()) ?? _emptyTree; | |
| 360 | + if (json == since) return null; | |
| 361 | + return UiFrame(json, _treeFrom(json)); | |
| 362 | +} | |
| 310 | 363 | |
| 311 | 364 | /// Apply an event and get the tree it produced. |
| 312 | 365 | /// |
| 313 | 366 | /// One call rather than dispatch-then-render, and not to save a crossing: it |
| 314 | 367 | /// makes the pair atomic, so there is no window in which Dart could render a |
| 315 | 368 | /// state nothing asked for. |
| 316 | -UiNode dispatch(String id, [String value = '']) { | |
| 317 | - final f = _lib.lookupFunction<_Str1Native, _Str1Dart>('frq_ui_dispatch'); | |
| 369 | +UiNode dispatch(String id, [String value = '']) => dispatchFrame(id, value).tree; | |
| 370 | + | |
| 371 | +/// As [dispatch], but keeping the JSON so the poll loop can compare against | |
| 372 | +/// it without re-stringifying the tree it just built. | |
| 373 | +UiFrame dispatchFrame(String id, [String value = '']) { | |
| 318 | 374 | final a = _toC(jsonEncode({'id': id, 'value': value})); |
| 319 | 375 | try { |
| 320 | - return _treeFrom(_takeString(f(a))); | |
| 376 | + final json = _takeString(_uiDispatch(a)) ?? '{"tag":"vbox"}'; | |
| 377 | + return UiFrame(json, _treeFrom(json)); | |
| 321 | 378 | } finally { |
| 322 | 379 | _freeArg(a); |
| 323 | 380 | } |
| @@ -325,9 +382,7 @@ UiNode dispatch(String id, [String value = '']) { | ||
| 325 | 382 | |
| 326 | 383 | /// Fill a room with a representative conversation, so a test can lay the chat |
| 327 | 384 | /// screen out without a server. See the Nim side for why it exists. |
| 328 | -void demoUi() => | |
| 329 | - _lib.lookupFunction<_VoidNative, _VoidDart>('frq_ui_demo')(); | |
| 385 | +void demoUi() => _uiDemo(); | |
| 330 | 386 | |
| 331 | 387 | /// Back to a fresh state, for a caller that wants a known starting point. |
| 332 | -void resetUi() => | |
| 333 | - _lib.lookupFunction<_VoidNative, _VoidDart>('frq_ui_reset')(); | |
| 388 | +void resetUi() => _uiReset(); | |
| @@ -91,7 +91,27 @@ final DynamicLibrary _lib = () { | |||
| 91 | return lib; | 91 | return lib; |
| 92 | }(); | 92 | }(); |
| 93 | 93 | ||
| 94 | +// Every entry point resolved once, here, rather than on each call. | ||
| 95 | +// | ||
| 96 | +// `lookupFunction` is a dlsym plus a freshly built trampoline closure every | ||
| 97 | +// time it runs. At 10Hz for `poll` and once per keystroke for `dispatch` that | ||
| 98 | +// is measurable and, more to the point, free to avoid — these are `final`, so | ||
| 99 | +// they cost one lookup for the life of the process. | ||
| 94 | final _free = _lib.lookupFunction<_FreeNative, _FreeDart>('frq_free'); | 100 | final _free = _lib.lookupFunction<_FreeNative, _FreeDart>('frq_free'); |
| 101 | +final _version = _lib.lookupFunction<_VersionNative, _VersionDart>('frq_version'); | ||
| 102 | +final _tagValue = _lib.lookupFunction<_Str2Native, _Str2Dart>('frq_irc_tag_value'); | ||
| 103 | +final _traceFn = _lib.lookupFunction<_Str2Native, _Str2Dart>('frq_trace'); | ||
| 104 | +final _connOpen = _lib.lookupFunction<_ConnOpenNative, _ConnOpenDart>('frq_conn_open'); | ||
| 105 | +final _connSend = _lib.lookupFunction<_Str1Native, _Str1Dart>('frq_conn_send'); | ||
| 106 | +final _connCloseFn = _lib.lookupFunction<_VoidNative, _VoidDart>('frq_conn_close'); | ||
| 107 | +final _connRecvFn = _lib.lookupFunction<_Str0Native, _Str0Dart>('frq_conn_recv'); | ||
| 108 | +final _connEventFn = _lib.lookupFunction<_Str0Native, _Str0Dart>('frq_conn_event'); | ||
| 109 | +final _uiRender = _lib.lookupFunction<_Str0Native, _Str0Dart>('frq_ui_render'); | ||
| 110 | +final _uiPoll = _lib.lookupFunction<_Str0Native, _Str0Dart>('frq_ui_poll'); | ||
| 111 | +final _uiDispatch = _lib.lookupFunction<_Str1Native, _Str1Dart>('frq_ui_dispatch'); | ||
| 112 | +final _uiDemo = _lib.lookupFunction<_VoidNative, _VoidDart>('frq_ui_demo'); | ||
| 113 | +final _uiReset = _lib.lookupFunction<_VoidNative, _VoidDart>('frq_ui_reset'); | ||
| 114 | +final _str1 = <String, _Str1Dart>{}; | ||
| 95 | 115 | ||
| 96 | /// The bytes at [p] as a string, with [p] freed afterwards. Null in, null out. | 116 | /// The bytes at [p] as a string, with [p] freed afterwards. Null in, null out. |
| 97 | String? _takeString(Pointer<Uint8> p) { | 117 | String? _takeString(Pointer<Uint8> p) { |
| @@ -133,9 +153,26 @@ final _malloc = _libc | |||
| 133 | final _freeArg = | 153 | final _freeArg = |
| 134 | _libc.lookupFunction<Void Function(Pointer<Uint8>), void Function(Pointer<Uint8>)>('free'); | 154 | _libc.lookupFunction<Void Function(Pointer<Uint8>), void Function(Pointer<Uint8>)>('free'); |
| 135 | 155 | ||
| 156 | +/// Call a two-strings-in, one-string-out entry point. | ||
| 157 | +/// | ||
| 158 | +/// The mirror of [_call1], and it exists for the same reason: the | ||
| 159 | +/// `_toC`/`try`/`finally`/`_freeArg` dance is four lines of ownership | ||
| 160 | +/// bookkeeping that no call site should repeat. | ||
| 161 | +String? _call2(_Str2Dart f, String x, String y) { | ||
| 162 | + final a = _toC(x); | ||
| 163 | + final b = _toC(y); | ||
| 164 | + try { | ||
| 165 | + return _takeString(f(a, b)); | ||
| 166 | + } finally { | ||
| 167 | + _freeArg(a); | ||
| 168 | + _freeArg(b); | ||
| 169 | + } | ||
| 170 | +} | ||
| 171 | + | ||
| 136 | /// Call a one-string-in, one-string-out entry point. | 172 | /// Call a one-string-in, one-string-out entry point. |
| 137 | String? _call1(String symbol, String arg) { | 173 | String? _call1(String symbol, String arg) { |
| 138 | - final f = _lib.lookupFunction<_Str1Native, _Str1Dart>(symbol); | 174 | + final f = _str1.putIfAbsent( |
| 175 | + symbol, () => _lib.lookupFunction<_Str1Native, _Str1Dart>(symbol)); | ||
| 139 | final a = _toC(arg); | 176 | final a = _toC(arg); |
| 140 | try { | 177 | try { |
| 141 | return _takeString(f(a)); | 178 | return _takeString(f(a)); |
| @@ -150,7 +187,7 @@ String? _call1(String symbol, String arg) { | |||
| 150 | /// is the one it was built against. Static storage on the Nim side: the one | 187 | /// is the one it was built against. Static storage on the Nim side: the one |
| 151 | /// return value that is NOT freed. | 188 | /// return value that is NOT freed. |
| 152 | String get version { | 189 | String get version { |
| 153 | - final p = _lib.lookupFunction<_VersionNative, _VersionDart>('frq_version')(); | 190 | + final p = _version(); |
| 154 | var len = 0; | 191 | var len = 0; |
| 155 | while (p[len] != 0) { | 192 | while (p[len] != 0) { |
| 156 | len++; | 193 | len++; |
| @@ -172,15 +209,7 @@ Map<String, dynamic> parseLine(String line) { | |||
| 172 | /// One IRCv3 tag's value, unescaped — null where the tag is absent OR empty, | 209 | /// One IRCv3 tag's value, unescaped — null where the tag is absent OR empty, |
| 173 | /// which IRCv3 says are the same thing. | 210 | /// which IRCv3 says are the same thing. |
| 174 | String? tagValue(String tags, String key) { | 211 | String? tagValue(String tags, String key) { |
| 175 | - final f = _lib.lookupFunction<_Str2Native, _Str2Dart>('frq_irc_tag_value'); | 212 | + return _call2(_tagValue, tags, key); |
| 176 | - final a = _toC(tags); | ||
| 177 | - final b = _toC(key); | ||
| 178 | - try { | ||
| 179 | - return _takeString(f(a, b)); | ||
| 180 | - } finally { | ||
| 181 | - _freeArg(a); | ||
| 182 | - _freeArg(b); | ||
| 183 | - } | ||
| 184 | } | 213 | } |
| 185 | 214 | ||
| 186 | String unescapeTag(String v) => _call1('frq_irc_unescape_tag', v) ?? ''; | 215 | String unescapeTag(String v) => _call1('frq_irc_unescape_tag', v) ?? ''; |
| @@ -194,15 +223,7 @@ String nickOf(String prefix) => _call1('frq_irc_nick_of', prefix) ?? ''; | |||
| 194 | /// Log through the Nim core's trace facility, so `FRQ_TRACE=1` gives one | 223 | /// Log through the Nim core's trace facility, so `FRQ_TRACE=1` gives one |
| 195 | /// interleaved story rather than two half-ones in different places. | 224 | /// interleaved story rather than two half-ones in different places. |
| 196 | void trace(String topic, String msg) { | 225 | void trace(String topic, String msg) { |
| 197 | - final f = _lib.lookupFunction<_Str2Native, _Str2Dart>('frq_trace'); | 226 | + _call2(_traceFn, topic, msg); |
| 198 | - final a = _toC(topic); | ||
| 199 | - final b = _toC(msg); | ||
| 200 | - try { | ||
| 201 | - f(a, b); | ||
| 202 | - } finally { | ||
| 203 | - _freeArg(a); | ||
| 204 | - _freeArg(b); | ||
| 205 | - } | ||
| 206 | } | 227 | } |
| 207 | 228 | ||
| 208 | // --------------------------------------------------------------- transport | 229 | // --------------------------------------------------------------- transport |
| @@ -217,7 +238,7 @@ typedef _ConnOpenDart = void Function(Pointer<Uint8>, int, int); | |||
| 217 | /// Dial. Non-blocking: the socket runs on a Nim thread and progress arrives | 238 | /// Dial. Non-blocking: the socket runs on a Nim thread and progress arrives |
| 218 | /// through [connEvent]. | 239 | /// through [connEvent]. |
| 219 | void connOpen(String host, int port, {bool tls = true}) { | 240 | void connOpen(String host, int port, {bool tls = true}) { |
| 220 | - final f = _lib.lookupFunction<_ConnOpenNative, _ConnOpenDart>('frq_conn_open'); | 241 | + final f = _connOpen; |
| 221 | final a = _toC(host); | 242 | final a = _toC(host); |
| 222 | try { | 243 | try { |
| 223 | f(a, port, tls ? 1 : 0); | 244 | f(a, port, tls ? 1 : 0); |
| @@ -228,7 +249,7 @@ void connOpen(String host, int port, {bool tls = true}) { | |||
| 228 | 249 | ||
| 229 | /// Queue a line. The transport adds the CRLF. | 250 | /// Queue a line. The transport adds the CRLF. |
| 230 | void connSend(String line) { | 251 | void connSend(String line) { |
| 231 | - final f = _lib.lookupFunction<_Str1Native, _Str1Dart>('frq_conn_send'); | 252 | + final f = _connSend; |
| 232 | final a = _toC(line); | 253 | final a = _toC(line); |
| 233 | try { | 254 | try { |
| 234 | f(a); | 255 | f(a); |
| @@ -237,16 +258,13 @@ void connSend(String line) { | |||
| 237 | } | 258 | } |
| 238 | } | 259 | } |
| 239 | 260 | ||
| 240 | -void connClose() => | 261 | +void connClose() => _connCloseFn(); |
| 241 | - _lib.lookupFunction<_VoidNative, _VoidDart>('frq_conn_close')(); | ||
| 242 | 262 | ||
| 243 | /// The next line, or null when none is waiting. Never blocks. | 263 | /// The next line, or null when none is waiting. Never blocks. |
| 244 | -String? connRecv() => _takeString( | 264 | +String? connRecv() => _takeString(_connRecvFn()); |
| 245 | - _lib.lookupFunction<_Str0Native, _Str0Dart>('frq_conn_recv')()); | ||
| 246 | 265 | ||
| 247 | /// The next transport event — `open`, `close: …`, `error: …` — or null. | 266 | /// The next transport event — `open`, `close: …`, `error: …` — or null. |
| 248 | -String? connEvent() => _takeString( | 267 | +String? connEvent() => _takeString(_connEventFn()); |
| 249 | - _lib.lookupFunction<_Str0Native, _Str0Dart>('frq_conn_event')()); | ||
| 250 | 268 | ||
| 251 | 269 | ||
| 252 | // ---------------------------------------------------------------- the UI | 270 | // ---------------------------------------------------------------- the UI |
| @@ -293,7 +311,23 @@ class UiNode { | |||
| 293 | } | 311 | } |
| 294 | 312 | ||
| 295 | UiNode _treeFrom(String? json) => | 313 | UiNode _treeFrom(String? json) => |
| 296 | - UiNode.fromJson(jsonDecode(json ?? '{"tag":"vbox"}') as Map<String, dynamic>); | 314 | + UiNode.fromJson(jsonDecode(json ?? _emptyTree) as Map<String, dynamic>); |
| 315 | + | ||
| 316 | +/// A tree and the JSON it came from. | ||
| 317 | +/// | ||
| 318 | +/// The raw string is kept because it is the cheapest possible change | ||
| 319 | +/// detector: Nim already produced it, and comparing two strings is free | ||
| 320 | +/// beside decoding one. The renderer polls ten times a second and the answer | ||
| 321 | +/// is almost always "nothing changed" — doing a `jsonDecode` and two | ||
| 322 | +/// recursive `toString()`s to discover that was most of the idle cost of the | ||
| 323 | +/// app in a busy room. | ||
| 324 | +const _emptyTree = '{"tag":"vbox"}'; | ||
| 325 | + | ||
| 326 | +class UiFrame { | ||
| 327 | + final String json; | ||
| 328 | + final UiNode tree; | ||
| 329 | + const UiFrame(this.json, this.tree); | ||
| 330 | +} | ||
| 297 | 331 | ||
| 298 | /// The current screen. | 332 | /// The current screen. |
| 299 | /// | 333 | /// |
| @@ -301,23 +335,46 @@ UiNode _treeFrom(String? json) => | |||
| 301 | /// no [dispatch] between can differ when a line arrived in the gap. That is how | 335 | /// no [dispatch] between can differ when a line arrived in the gap. That is how |
| 302 | /// the room fills, and why the renderer polls. | 336 | /// the room fills, and why the renderer polls. |
| 303 | UiNode render() => _treeFrom( | 337 | UiNode render() => _treeFrom( |
| 304 | - _takeString(_lib.lookupFunction<_Str0Native, _Str0Dart>('frq_ui_render')())); | 338 | + _takeString(_uiRender())); |
| 305 | 339 | ||
| 306 | /// The tree, asked for because time passed rather than because anything | 340 | /// The tree, asked for because time passed rather than because anything |
| 307 | /// happened. Same work as [render]; named for what the caller means. | 341 | /// happened. Same work as [render]; named for what the caller means. |
| 308 | UiNode poll() => _treeFrom( | 342 | UiNode poll() => _treeFrom( |
| 309 | - _takeString(_lib.lookupFunction<_Str0Native, _Str0Dart>('frq_ui_poll')())); | 343 | + _takeString(_uiPoll())); |
| 344 | + | ||
| 345 | +/// The current screen, with the JSON it came from. The starting point for | ||
| 346 | +/// [pollIfChanged]. | ||
| 347 | +UiFrame renderFrame() { | ||
| 348 | + final json = _takeString(_uiRender()) ?? _emptyTree; | ||
| 349 | + return UiFrame(json, _treeFrom(json)); | ||
| 350 | +} | ||
| 351 | + | ||
| 352 | +/// The tree, decoded only when it differs from [since] — otherwise null, | ||
| 353 | +/// meaning "the screen you already have is current". | ||
| 354 | +/// | ||
| 355 | +/// This is what the renderer polls with. The comparison is the JSON Nim | ||
| 356 | +/// already produced, so an unchanged frame costs one string compare rather | ||
| 357 | +/// than a decode and two recursive `toString()`s. | ||
| 358 | +UiFrame? pollIfChanged(String since) { | ||
| 359 | + final json = _takeString(_uiPoll()) ?? _emptyTree; | ||
| 360 | + if (json == since) return null; | ||
| 361 | + return UiFrame(json, _treeFrom(json)); | ||
| 362 | +} | ||
| 310 | 363 | ||
| 311 | /// Apply an event and get the tree it produced. | 364 | /// Apply an event and get the tree it produced. |
| 312 | /// | 365 | /// |
| 313 | /// One call rather than dispatch-then-render, and not to save a crossing: it | 366 | /// One call rather than dispatch-then-render, and not to save a crossing: it |
| 314 | /// makes the pair atomic, so there is no window in which Dart could render a | 367 | /// makes the pair atomic, so there is no window in which Dart could render a |
| 315 | /// state nothing asked for. | 368 | /// state nothing asked for. |
| 316 | -UiNode dispatch(String id, [String value = '']) { | 369 | +UiNode dispatch(String id, [String value = '']) => dispatchFrame(id, value).tree; |
| 317 | - final f = _lib.lookupFunction<_Str1Native, _Str1Dart>('frq_ui_dispatch'); | 370 | + |
| 371 | +/// As [dispatch], but keeping the JSON so the poll loop can compare against | ||
| 372 | +/// it without re-stringifying the tree it just built. | ||
| 373 | +UiFrame dispatchFrame(String id, [String value = '']) { | ||
| 318 | final a = _toC(jsonEncode({'id': id, 'value': value})); | 374 | final a = _toC(jsonEncode({'id': id, 'value': value})); |
| 319 | try { | 375 | try { |
| 320 | - return _treeFrom(_takeString(f(a))); | 376 | + final json = _takeString(_uiDispatch(a)) ?? '{"tag":"vbox"}'; |
| 377 | + return UiFrame(json, _treeFrom(json)); | ||
| 321 | } finally { | 378 | } finally { |
| 322 | _freeArg(a); | 379 | _freeArg(a); |
| 323 | } | 380 | } |
| @@ -325,9 +382,7 @@ UiNode dispatch(String id, [String value = '']) { | |||
| 325 | 382 | ||
| 326 | /// Fill a room with a representative conversation, so a test can lay the chat | 383 | /// Fill a room with a representative conversation, so a test can lay the chat |
| 327 | /// screen out without a server. See the Nim side for why it exists. | 384 | /// screen out without a server. See the Nim side for why it exists. |
| 328 | -void demoUi() => | 385 | +void demoUi() => _uiDemo(); |
| 329 | - _lib.lookupFunction<_VoidNative, _VoidDart>('frq_ui_demo')(); | ||
| 330 | 386 | ||
| 331 | /// Back to a fresh state, for a caller that wants a known starting point. | 387 | /// Back to a fresh state, for a caller that wants a known starting point. |
| 332 | -void resetUi() => | 388 | +void resetUi() => _uiReset(); |
| 333 | - _lib.lookupFunction<_VoidNative, _VoidDart>('frq_ui_reset')(); | ||
modified
flutter/lib/main_nim.dart +26 -1 | @@ -10,7 +10,32 @@ | ||
| 10 | 10 | /// copy, same fields, same stable wrappers. What is not yet here is the emoji |
| 11 | 11 | /// picker, the overview strip, the lightbox and the profile card — see |
| 12 | 12 | /// `nim/README.md` for what is done and what is not. |
| 13 | +library; | |
| 14 | + | |
| 15 | +import 'dart:io'; | |
| 16 | + | |
| 13 | 17 | import 'package:flutter/material.dart'; |
| 18 | +import 'package:frq_core/frq_core.dart' as core; | |
| 19 | + | |
| 14 | 20 | import 'nim_renderer.dart'; |
| 15 | 21 | |
| 16 | -void main() => runApp(const NimApp()); | |
| 22 | +void main() { | |
| 23 | + // FRQ_AUTOCONNECT presses Connect at startup, and FRQ_NICK overrides the | |
| 24 | + // nickname — a Wayland window cannot be clicked from a script, so without | |
| 25 | + // this the only way to check that the app gets past the connect screen is | |
| 26 | + // to sit in front of it. | |
| 27 | + // | |
| 28 | + // Read here rather than in Nim, and dispatched through the same door the | |
| 29 | + // button uses. It lived in `currentTree()` before, which meant the function | |
| 30 | + // whose job is "serialise the current screen" opened a socket on its first | |
| 31 | + // call depending on the process environment. | |
| 32 | + final env = Platform.environment; | |
| 33 | + final want = env['FRQ_AUTOCONNECT'] ?? ''; | |
| 34 | + if (want.isNotEmpty && want != '0') { | |
| 35 | + final nick = env['FRQ_NICK'] ?? ''; | |
| 36 | + if (nick.isNotEmpty) core.dispatch('nick.change', nick); | |
| 37 | + core.dispatch('connect'); | |
| 38 | + } | |
| 39 | + | |
| 40 | + runApp(const NimApp()); | |
| 41 | +} | |
| @@ -10,7 +10,32 @@ | |||
| 10 | /// copy, same fields, same stable wrappers. What is not yet here is the emoji | 10 | /// copy, same fields, same stable wrappers. What is not yet here is the emoji |
| 11 | /// picker, the overview strip, the lightbox and the profile card — see | 11 | /// picker, the overview strip, the lightbox and the profile card — see |
| 12 | /// `nim/README.md` for what is done and what is not. | 12 | /// `nim/README.md` for what is done and what is not. |
| 13 | +library; | ||
| 14 | + | ||
| 15 | +import 'dart:io'; | ||
| 16 | + | ||
| 13 | import 'package:flutter/material.dart'; | 17 | import 'package:flutter/material.dart'; |
| 18 | +import 'package:frq_core/frq_core.dart' as core; | ||
| 19 | + | ||
| 14 | import 'nim_renderer.dart'; | 20 | import 'nim_renderer.dart'; |
| 15 | 21 | ||
| 16 | -void main() => runApp(const NimApp()); | 22 | +void main() { |
| 23 | + // FRQ_AUTOCONNECT presses Connect at startup, and FRQ_NICK overrides the | ||
| 24 | + // nickname — a Wayland window cannot be clicked from a script, so without | ||
| 25 | + // this the only way to check that the app gets past the connect screen is | ||
| 26 | + // to sit in front of it. | ||
| 27 | + // | ||
| 28 | + // Read here rather than in Nim, and dispatched through the same door the | ||
| 29 | + // button uses. It lived in `currentTree()` before, which meant the function | ||
| 30 | + // whose job is "serialise the current screen" opened a socket on its first | ||
| 31 | + // call depending on the process environment. | ||
| 32 | + final env = Platform.environment; | ||
| 33 | + final want = env['FRQ_AUTOCONNECT'] ?? ''; | ||
| 34 | + if (want.isNotEmpty && want != '0') { | ||
| 35 | + final nick = env['FRQ_NICK'] ?? ''; | ||
| 36 | + if (nick.isNotEmpty) core.dispatch('nick.change', nick); | ||
| 37 | + core.dispatch('connect'); | ||
| 38 | + } | ||
| 39 | + | ||
| 40 | + runApp(const NimApp()); | ||
| 41 | +} | ||
modified
flutter/lib/main_nim_app.dart +2 -0 | @@ -3,4 +3,6 @@ | ||
| 3 | 3 | /// Every screen, every cell and every action exactly as they are; only the |
| 4 | 4 | /// transport underneath them is Nim. `frq.main-nim` is `frq.main` with one |
| 5 | 5 | /// line changed. |
| 6 | +library; | |
| 7 | + | |
| 6 | 8 | export "cljd-out/frq/main-nim.dart" show main; |
| @@ -3,4 +3,6 @@ | |||
| 3 | /// Every screen, every cell and every action exactly as they are; only the | 3 | /// Every screen, every cell and every action exactly as they are; only the |
| 4 | /// transport underneath them is Nim. `frq.main-nim` is `frq.main` with one | 4 | /// transport underneath them is Nim. `frq.main-nim` is `frq.main` with one |
| 5 | /// line changed. | 5 | /// line changed. |
| 6 | +library; | ||
| 7 | + | ||
| 6 | export "cljd-out/frq/main-nim.dart" show main; | 8 | export "cljd-out/frq/main-nim.dart" show main; |
modified
flutter/lib/nim_renderer.dart +55 -46 | @@ -27,7 +27,8 @@ class NimApp extends StatefulWidget { | ||
| 27 | 27 | } |
| 28 | 28 | |
| 29 | 29 | class _NimAppState extends State<NimApp> { |
| 30 | - late core.UiNode _tree = core.render(); | |
| 30 | + late core.UiFrame _frame = core.renderFrame(); | |
| 31 | + core.UiNode get _tree => _frame.tree; | |
| 31 | 32 | Timer? _poll; |
| 32 | 33 | |
| 33 | 34 | // One controller and one focus node per keyed entry, kept across rebuilds. |
| @@ -52,16 +53,18 @@ class _NimAppState extends State<NimApp> { | ||
| 52 | 53 | // Polling, because the socket lives on a Nim thread and there is no |
| 53 | 54 | // callback into Dart. At ~70µs a render a 100ms timer costs nothing. |
| 54 | 55 | _poll = Timer.periodic(const Duration(milliseconds: 100), (_) { |
| 55 | - final next = core.poll(); | |
| 56 | - if (next.toString() != _tree.toString()) { | |
| 57 | - setState(() => _tree = next); | |
| 58 | - } | |
| 56 | + // Compared as the JSON Nim already produced, and decoded only when it | |
| 57 | + // differs. Stringifying both trees to answer "did anything change" was | |
| 58 | + // ~1MB of string churn per poll in a busy room, ten times a second, for | |
| 59 | + // an answer that is almost always no. | |
| 60 | + final next = core.pollIfChanged(_frame.json); | |
| 61 | + if (next != null) setState(() => _frame = next); | |
| 59 | 62 | }); |
| 60 | 63 | } |
| 61 | 64 | |
| 62 | 65 | void _send(String id, [String value = '']) { |
| 63 | 66 | if (id.isEmpty) return; |
| 64 | - setState(() => _tree = core.dispatch(id, value)); | |
| 67 | + setState(() => _frame = core.dispatchFrame(id, value)); | |
| 65 | 68 | if (id == 'send') _focus['draft']?.requestFocus(); |
| 66 | 69 | } |
| 67 | 70 | |
| @@ -153,6 +156,15 @@ class _NimAppState extends State<NimApp> { | ||
| 153 | 156 | /// layout, and every box under it is then asked to hit-test without ever |
| 154 | 157 | /// having been laid out. The chats screen did exactly that — two unsized |
| 155 | 158 | /// entries in an `hbox`, which is a Wrap. |
| 159 | + /// What an unsized entry or a stranded scroll falls back to. | |
| 160 | + /// | |
| 161 | + /// Both are only reachable when the tree has put one outside a Flex, which | |
| 162 | + /// is a tree bug rather than a rendering choice. The numbers exist so that | |
| 163 | + /// bug renders as something a person can see and a test can catch, not so | |
| 164 | + /// that it renders correctly. | |
| 165 | + static const _unsizedEntry = 320.0; | |
| 166 | + static const _strandedScroll = 400.0; | |
| 167 | + | |
| 156 | 168 | static const _noAxis = ''; |
| 157 | 169 | static const _row = 'row'; |
| 158 | 170 | static const _column = 'column'; |
| @@ -164,10 +176,24 @@ class _NimAppState extends State<NimApp> { | ||
| 164 | 176 | // What this node's own children are being built into. |
| 165 | 177 | final childAxis = switch (n.tag) { |
| 166 | 178 | 'page' || 'vbox' || 'card' || 'scroll' || 'dialog' => _column, |
| 179 | + // Wrapping unless the row says otherwise. Flipping this default was | |
| 180 | + // tried and reverted: only 4 of 15 `hbox` call sites state `wrap` at | |
| 181 | + // all, so the other 11 became Rows and overflowed — the tree's habit is | |
| 182 | + // to wrap, and the default has to match it. | |
| 167 | 183 | 'hbox' => n.prop('wrap', true) ? _noAxis : _row, |
| 168 | 184 | _ => _noAxis, |
| 169 | 185 | }; |
| 170 | - final kids = n.children.map((c) => _build(c, childAxis)).toList(); | |
| 186 | + // A paragraph's children are spans, not widgets — building them as | |
| 187 | + // widgets and throwing them away is what the `inline` special case did. | |
| 188 | + final kids = n.tag == 'paragraph' | |
| 189 | + ? const <Widget>[] | |
| 190 | + : n.children.map((c) => _build(c, childAxis)).toList(); | |
| 191 | + | |
| 192 | + // One rule for "take the remaining main-axis extent", stated by the node | |
| 193 | + // that expands. It used to be three: a vbox prop, a scroll with no | |
| 194 | + // height, and a row peering at its children's props to infer it. | |
| 195 | + Widget expanded(Widget w) => | |
| 196 | + (n.prop('expand', false) && flex) ? Expanded(child: w) : w; | |
| 171 | 197 | |
| 172 | 198 | switch (n.tag) { |
| 173 | 199 | case 'page': |
| @@ -196,24 +222,14 @@ class _NimAppState extends State<NimApp> { | ||
| 196 | 222 | col = _margins(n, col); |
| 197 | 223 | final w = _d(n.props['widthRequest'], 0); |
| 198 | 224 | if (w > 0) col = SizedBox(width: w, child: col); |
| 199 | - // `fillHeight` is what keeps the compose bar at the bottom instead | |
| 200 | - // of wherever the backlog happens to end — but only a Flex can be | |
| 201 | - // told to expand into. | |
| 202 | - return (n.prop('fillHeight', false) && flex) | |
| 203 | - ? Expanded(child: col) | |
| 204 | - : col; | |
| 225 | + return expanded(col); | |
| 205 | 226 | } |
| 206 | 227 | |
| 207 | - // A paragraph: the words and the links of one message, wrapping as text | |
| 208 | - // rather than as boxes. | |
| 209 | - // | |
| 210 | - // NOT a Wrap, which is what this was. Children of a Wrap are given | |
| 228 | + // Prose with links in it. NOT a Wrap: children of a Wrap are given | |
| 211 | 229 | // unbounded width, so a long URL or a long word can never wrap — it |
| 212 | 230 | // overflows, the layout fails, and every box under it is then hit-tested |
| 213 | - // having never been laid out. That is the pile of "Cannot hit test" | |
| 214 | - // errors the chat screen produced. Spans in one RichText wrap the way | |
| 215 | - // the Clojure's `:inline` row always meant. | |
| 216 | - case 'hbox' when n.prop('inline', false): | |
| 231 | + // having never been laid out. Spans in one RichText wrap properly. | |
| 232 | + case 'paragraph': | |
| 217 | 233 | return Text.rich( |
| 218 | 234 | TextSpan(children: n.children.map(_span).toList()), |
| 219 | 235 | softWrap: true, |
| @@ -228,28 +244,20 @@ class _NimAppState extends State<NimApp> { | ||
| 228 | 244 | final wrapping = n.prop('wrap', true); |
| 229 | 245 | final align = n.prop('align', 'center'); |
| 230 | 246 | if (!wrapping) { |
| 231 | - // A child asking to fill the height gets it from the row's cross | |
| 232 | - // axis, not from an Expanded — Expanded in a Row is about width. | |
| 233 | - // | |
| 234 | - // But stretch needs a bounded height to stretch to, and a Row in a | |
| 235 | - // Column has none of its own: it is as tall as its tallest child. | |
| 236 | - // So the row that holds a filling child has to take the column's | |
| 237 | - // remaining height itself, or the stretch resolves to infinity and | |
| 238 | - // the assertion reads `BoxConstraints forces an infinite height`. | |
| 239 | - final stretches = | |
| 240 | - n.children.any((c) => c.prop('fillHeight', false)); | |
| 247 | + // An expanding row stretches on its cross axis, which is where a | |
| 248 | + // child's height comes from — Expanded in a Row is about width. | |
| 249 | + // Stretch needs a bounded height, and `expanded()` below is what | |
| 250 | + // gives the row one; without it the stretch resolves to infinity. | |
| 251 | + final fills = n.prop('expand', false); | |
| 241 | 252 | final row = Row( |
| 242 | - crossAxisAlignment: stretches | |
| 253 | + crossAxisAlignment: fills | |
| 243 | 254 | ? CrossAxisAlignment.stretch |
| 244 | 255 | : (align == 'end' |
| 245 | 256 | ? CrossAxisAlignment.end |
| 246 | 257 | : CrossAxisAlignment.center), |
| 247 | 258 | children: _spaced(kids, spacing, vertical: false), |
| 248 | 259 | ); |
| 249 | - if (stretches && axis == _column) { | |
| 250 | - return Expanded(child: _margins(n, row)); | |
| 251 | - } | |
| 252 | - return _margins(n, row); | |
| 260 | + return expanded(_margins(n, row)); | |
| 253 | 261 | } |
| 254 | 262 | return _margins( |
| 255 | 263 | n, |
| @@ -465,7 +473,7 @@ class _NimAppState extends State<NimApp> { | ||
| 465 | 473 | radius: size / 2, |
| 466 | 474 | backgroundColor: t.component, |
| 467 | 475 | backgroundImage: provider, |
| 468 | - onBackgroundImageError: provider == null ? null : (_, __) {}, | |
| 476 | + onBackgroundImageError: provider == null ? null : (_, _) {}, | |
| 469 | 477 | child: provider == null |
| 470 | 478 | ? Text( |
| 471 | 479 | fallback.isNotEmpty |
| @@ -496,7 +504,7 @@ class _NimAppState extends State<NimApp> { | ||
| 496 | 504 | // throws during the build, and an exception in a build is a red |
| 497 | 505 | // screen for the whole conversation rather than a gap where one |
| 498 | 506 | // picture was. |
| 499 | - errorBuilder: (_, __, ___) => const SizedBox.shrink(), | |
| 507 | + errorBuilder: (_, _, _) => const SizedBox.shrink(), | |
| 500 | 508 | ); |
| 501 | 509 | if (maxW > 0 || maxH > 0) { |
| 502 | 510 | img = ConstrainedBox( |
| @@ -551,7 +559,7 @@ class _NimAppState extends State<NimApp> { | ||
| 551 | 559 | // took the whole screen down rather than one field. |
| 552 | 560 | return axis == _row |
| 553 | 561 | ? Expanded(child: field) |
| 554 | - : SizedBox(width: 320, child: field); | |
| 562 | + : SizedBox(width: _unsizedEntry, child: field); | |
| 555 | 563 | } |
| 556 | 564 | |
| 557 | 565 | case 'scroll': |
| @@ -564,12 +572,13 @@ class _NimAppState extends State<NimApp> { | ||
| 564 | 572 | children: _spaced(kids, spacing, vertical: true)), |
| 565 | 573 | ); |
| 566 | 574 | body = Scrollbar(child: body); |
| 567 | - final h = _d(n.props['height'], 0); | |
| 568 | - if (h > 0) return SizedBox(height: h, child: body); | |
| 569 | - // No fixed height: take what the column has left, where there is a | |
| 570 | - // column. `reserve` is the Clojure's way of saying the same thing to | |
| 571 | - // a backend that could not do this, and is ignored here on purpose. | |
| 572 | - return flex ? Expanded(child: body) : SizedBox(height: 400, child: body); | |
| 575 | + // A scroll takes what the column has left. Outside a Flex there is | |
| 576 | + // nothing to take, and the tree is malformed — `_strandedScroll` is | |
| 577 | + // a visible size rather than a correct one, so the layout tests see | |
| 578 | + // a screen instead of an exception. | |
| 579 | + return flex | |
| 580 | + ? Expanded(child: body) | |
| 581 | + : const SizedBox(height: _strandedScroll); | |
| 573 | 582 | } |
| 574 | 583 | |
| 575 | 584 | /// A panel over the screen rather than a screen of its own. |
| @@ -637,7 +646,7 @@ class _NimAppState extends State<NimApp> { | ||
| 637 | 646 | } |
| 638 | 647 | } |
| 639 | 648 | |
| 640 | - /// `margin`, `marginTop`, `marginBottom`, `marginRight` — the props the | |
| 649 | + /// `margin` and its four sides — the props the | |
| 641 | 650 | /// screens use to buy air without a wrapper each time. |
| 642 | 651 | Widget _margins(core.UiNode n, Widget child) { |
| 643 | 652 | final all = _d(n.props['margin'], 0); |
| @@ -27,7 +27,8 @@ class NimApp extends StatefulWidget { | |||
| 27 | } | 27 | } |
| 28 | 28 | ||
| 29 | class _NimAppState extends State<NimApp> { | 29 | class _NimAppState extends State<NimApp> { |
| 30 | - late core.UiNode _tree = core.render(); | 30 | + late core.UiFrame _frame = core.renderFrame(); |
| 31 | + core.UiNode get _tree => _frame.tree; | ||
| 31 | Timer? _poll; | 32 | Timer? _poll; |
| 32 | 33 | ||
| 33 | // One controller and one focus node per keyed entry, kept across rebuilds. | 34 | // One controller and one focus node per keyed entry, kept across rebuilds. |
| @@ -52,16 +53,18 @@ class _NimAppState extends State<NimApp> { | |||
| 52 | // Polling, because the socket lives on a Nim thread and there is no | 53 | // Polling, because the socket lives on a Nim thread and there is no |
| 53 | // callback into Dart. At ~70µs a render a 100ms timer costs nothing. | 54 | // callback into Dart. At ~70µs a render a 100ms timer costs nothing. |
| 54 | _poll = Timer.periodic(const Duration(milliseconds: 100), (_) { | 55 | _poll = Timer.periodic(const Duration(milliseconds: 100), (_) { |
| 55 | - final next = core.poll(); | 56 | + // Compared as the JSON Nim already produced, and decoded only when it |
| 56 | - if (next.toString() != _tree.toString()) { | 57 | + // differs. Stringifying both trees to answer "did anything change" was |
| 57 | - setState(() => _tree = next); | 58 | + // ~1MB of string churn per poll in a busy room, ten times a second, for |
| 58 | - } | 59 | + // an answer that is almost always no. |
| 60 | + final next = core.pollIfChanged(_frame.json); | ||
| 61 | + if (next != null) setState(() => _frame = next); | ||
| 59 | }); | 62 | }); |
| 60 | } | 63 | } |
| 61 | 64 | ||
| 62 | void _send(String id, [String value = '']) { | 65 | void _send(String id, [String value = '']) { |
| 63 | if (id.isEmpty) return; | 66 | if (id.isEmpty) return; |
| 64 | - setState(() => _tree = core.dispatch(id, value)); | 67 | + setState(() => _frame = core.dispatchFrame(id, value)); |
| 65 | if (id == 'send') _focus['draft']?.requestFocus(); | 68 | if (id == 'send') _focus['draft']?.requestFocus(); |
| 66 | } | 69 | } |
| 67 | 70 | ||
| @@ -153,6 +156,15 @@ class _NimAppState extends State<NimApp> { | |||
| 153 | /// layout, and every box under it is then asked to hit-test without ever | 156 | /// layout, and every box under it is then asked to hit-test without ever |
| 154 | /// having been laid out. The chats screen did exactly that — two unsized | 157 | /// having been laid out. The chats screen did exactly that — two unsized |
| 155 | /// entries in an `hbox`, which is a Wrap. | 158 | /// entries in an `hbox`, which is a Wrap. |
| 159 | + /// What an unsized entry or a stranded scroll falls back to. | ||
| 160 | + /// | ||
| 161 | + /// Both are only reachable when the tree has put one outside a Flex, which | ||
| 162 | + /// is a tree bug rather than a rendering choice. The numbers exist so that | ||
| 163 | + /// bug renders as something a person can see and a test can catch, not so | ||
| 164 | + /// that it renders correctly. | ||
| 165 | + static const _unsizedEntry = 320.0; | ||
| 166 | + static const _strandedScroll = 400.0; | ||
| 167 | + | ||
| 156 | static const _noAxis = ''; | 168 | static const _noAxis = ''; |
| 157 | static const _row = 'row'; | 169 | static const _row = 'row'; |
| 158 | static const _column = 'column'; | 170 | static const _column = 'column'; |
| @@ -164,10 +176,24 @@ class _NimAppState extends State<NimApp> { | |||
| 164 | // What this node's own children are being built into. | 176 | // What this node's own children are being built into. |
| 165 | final childAxis = switch (n.tag) { | 177 | final childAxis = switch (n.tag) { |
| 166 | 'page' || 'vbox' || 'card' || 'scroll' || 'dialog' => _column, | 178 | 'page' || 'vbox' || 'card' || 'scroll' || 'dialog' => _column, |
| 179 | + // Wrapping unless the row says otherwise. Flipping this default was | ||
| 180 | + // tried and reverted: only 4 of 15 `hbox` call sites state `wrap` at | ||
| 181 | + // all, so the other 11 became Rows and overflowed — the tree's habit is | ||
| 182 | + // to wrap, and the default has to match it. | ||
| 167 | 'hbox' => n.prop('wrap', true) ? _noAxis : _row, | 183 | 'hbox' => n.prop('wrap', true) ? _noAxis : _row, |
| 168 | _ => _noAxis, | 184 | _ => _noAxis, |
| 169 | }; | 185 | }; |
| 170 | - final kids = n.children.map((c) => _build(c, childAxis)).toList(); | 186 | + // A paragraph's children are spans, not widgets — building them as |
| 187 | + // widgets and throwing them away is what the `inline` special case did. | ||
| 188 | + final kids = n.tag == 'paragraph' | ||
| 189 | + ? const <Widget>[] | ||
| 190 | + : n.children.map((c) => _build(c, childAxis)).toList(); | ||
| 191 | + | ||
| 192 | + // One rule for "take the remaining main-axis extent", stated by the node | ||
| 193 | + // that expands. It used to be three: a vbox prop, a scroll with no | ||
| 194 | + // height, and a row peering at its children's props to infer it. | ||
| 195 | + Widget expanded(Widget w) => | ||
| 196 | + (n.prop('expand', false) && flex) ? Expanded(child: w) : w; | ||
| 171 | 197 | ||
| 172 | switch (n.tag) { | 198 | switch (n.tag) { |
| 173 | case 'page': | 199 | case 'page': |
| @@ -196,24 +222,14 @@ class _NimAppState extends State<NimApp> { | |||
| 196 | col = _margins(n, col); | 222 | col = _margins(n, col); |
| 197 | final w = _d(n.props['widthRequest'], 0); | 223 | final w = _d(n.props['widthRequest'], 0); |
| 198 | if (w > 0) col = SizedBox(width: w, child: col); | 224 | if (w > 0) col = SizedBox(width: w, child: col); |
| 199 | - // `fillHeight` is what keeps the compose bar at the bottom instead | 225 | + return expanded(col); |
| 200 | - // of wherever the backlog happens to end — but only a Flex can be | ||
| 201 | - // told to expand into. | ||
| 202 | - return (n.prop('fillHeight', false) && flex) | ||
| 203 | - ? Expanded(child: col) | ||
| 204 | - : col; | ||
| 205 | } | 226 | } |
| 206 | 227 | ||
| 207 | - // A paragraph: the words and the links of one message, wrapping as text | 228 | + // Prose with links in it. NOT a Wrap: children of a Wrap are given |
| 208 | - // rather than as boxes. | ||
| 209 | - // | ||
| 210 | - // NOT a Wrap, which is what this was. Children of a Wrap are given | ||
| 211 | // unbounded width, so a long URL or a long word can never wrap — it | 229 | // unbounded width, so a long URL or a long word can never wrap — it |
| 212 | // overflows, the layout fails, and every box under it is then hit-tested | 230 | // overflows, the layout fails, and every box under it is then hit-tested |
| 213 | - // having never been laid out. That is the pile of "Cannot hit test" | 231 | + // having never been laid out. Spans in one RichText wrap properly. |
| 214 | - // errors the chat screen produced. Spans in one RichText wrap the way | 232 | + case 'paragraph': |
| 215 | - // the Clojure's `:inline` row always meant. | ||
| 216 | - case 'hbox' when n.prop('inline', false): | ||
| 217 | return Text.rich( | 233 | return Text.rich( |
| 218 | TextSpan(children: n.children.map(_span).toList()), | 234 | TextSpan(children: n.children.map(_span).toList()), |
| 219 | softWrap: true, | 235 | softWrap: true, |
| @@ -228,28 +244,20 @@ class _NimAppState extends State<NimApp> { | |||
| 228 | final wrapping = n.prop('wrap', true); | 244 | final wrapping = n.prop('wrap', true); |
| 229 | final align = n.prop('align', 'center'); | 245 | final align = n.prop('align', 'center'); |
| 230 | if (!wrapping) { | 246 | if (!wrapping) { |
| 231 | - // A child asking to fill the height gets it from the row's cross | 247 | + // An expanding row stretches on its cross axis, which is where a |
| 232 | - // axis, not from an Expanded — Expanded in a Row is about width. | 248 | + // child's height comes from — Expanded in a Row is about width. |
| 233 | - // | 249 | + // Stretch needs a bounded height, and `expanded()` below is what |
| 234 | - // But stretch needs a bounded height to stretch to, and a Row in a | 250 | + // gives the row one; without it the stretch resolves to infinity. |
| 235 | - // Column has none of its own: it is as tall as its tallest child. | 251 | + final fills = n.prop('expand', false); |
| 236 | - // So the row that holds a filling child has to take the column's | ||
| 237 | - // remaining height itself, or the stretch resolves to infinity and | ||
| 238 | - // the assertion reads `BoxConstraints forces an infinite height`. | ||
| 239 | - final stretches = | ||
| 240 | - n.children.any((c) => c.prop('fillHeight', false)); | ||
| 241 | final row = Row( | 252 | final row = Row( |
| 242 | - crossAxisAlignment: stretches | 253 | + crossAxisAlignment: fills |
| 243 | ? CrossAxisAlignment.stretch | 254 | ? CrossAxisAlignment.stretch |
| 244 | : (align == 'end' | 255 | : (align == 'end' |
| 245 | ? CrossAxisAlignment.end | 256 | ? CrossAxisAlignment.end |
| 246 | : CrossAxisAlignment.center), | 257 | : CrossAxisAlignment.center), |
| 247 | children: _spaced(kids, spacing, vertical: false), | 258 | children: _spaced(kids, spacing, vertical: false), |
| 248 | ); | 259 | ); |
| 249 | - if (stretches && axis == _column) { | 260 | + return expanded(_margins(n, row)); |
| 250 | - return Expanded(child: _margins(n, row)); | ||
| 251 | - } | ||
| 252 | - return _margins(n, row); | ||
| 253 | } | 261 | } |
| 254 | return _margins( | 262 | return _margins( |
| 255 | n, | 263 | n, |
| @@ -465,7 +473,7 @@ class _NimAppState extends State<NimApp> { | |||
| 465 | radius: size / 2, | 473 | radius: size / 2, |
| 466 | backgroundColor: t.component, | 474 | backgroundColor: t.component, |
| 467 | backgroundImage: provider, | 475 | backgroundImage: provider, |
| 468 | - onBackgroundImageError: provider == null ? null : (_, __) {}, | 476 | + onBackgroundImageError: provider == null ? null : (_, _) {}, |
| 469 | child: provider == null | 477 | child: provider == null |
| 470 | ? Text( | 478 | ? Text( |
| 471 | fallback.isNotEmpty | 479 | fallback.isNotEmpty |
| @@ -496,7 +504,7 @@ class _NimAppState extends State<NimApp> { | |||
| 496 | // throws during the build, and an exception in a build is a red | 504 | // throws during the build, and an exception in a build is a red |
| 497 | // screen for the whole conversation rather than a gap where one | 505 | // screen for the whole conversation rather than a gap where one |
| 498 | // picture was. | 506 | // picture was. |
| 499 | - errorBuilder: (_, __, ___) => const SizedBox.shrink(), | 507 | + errorBuilder: (_, _, _) => const SizedBox.shrink(), |
| 500 | ); | 508 | ); |
| 501 | if (maxW > 0 || maxH > 0) { | 509 | if (maxW > 0 || maxH > 0) { |
| 502 | img = ConstrainedBox( | 510 | img = ConstrainedBox( |
| @@ -551,7 +559,7 @@ class _NimAppState extends State<NimApp> { | |||
| 551 | // took the whole screen down rather than one field. | 559 | // took the whole screen down rather than one field. |
| 552 | return axis == _row | 560 | return axis == _row |
| 553 | ? Expanded(child: field) | 561 | ? Expanded(child: field) |
| 554 | - : SizedBox(width: 320, child: field); | 562 | + : SizedBox(width: _unsizedEntry, child: field); |
| 555 | } | 563 | } |
| 556 | 564 | ||
| 557 | case 'scroll': | 565 | case 'scroll': |
| @@ -564,12 +572,13 @@ class _NimAppState extends State<NimApp> { | |||
| 564 | children: _spaced(kids, spacing, vertical: true)), | 572 | children: _spaced(kids, spacing, vertical: true)), |
| 565 | ); | 573 | ); |
| 566 | body = Scrollbar(child: body); | 574 | body = Scrollbar(child: body); |
| 567 | - final h = _d(n.props['height'], 0); | 575 | + // A scroll takes what the column has left. Outside a Flex there is |
| 568 | - if (h > 0) return SizedBox(height: h, child: body); | 576 | + // nothing to take, and the tree is malformed — `_strandedScroll` is |
| 569 | - // No fixed height: take what the column has left, where there is a | 577 | + // a visible size rather than a correct one, so the layout tests see |
| 570 | - // column. `reserve` is the Clojure's way of saying the same thing to | 578 | + // a screen instead of an exception. |
| 571 | - // a backend that could not do this, and is ignored here on purpose. | 579 | + return flex |
| 572 | - return flex ? Expanded(child: body) : SizedBox(height: 400, child: body); | 580 | + ? Expanded(child: body) |
| 581 | + : const SizedBox(height: _strandedScroll); | ||
| 573 | } | 582 | } |
| 574 | 583 | ||
| 575 | /// A panel over the screen rather than a screen of its own. | 584 | /// A panel over the screen rather than a screen of its own. |
| @@ -637,7 +646,7 @@ class _NimAppState extends State<NimApp> { | |||
| 637 | } | 646 | } |
| 638 | } | 647 | } |
| 639 | 648 | ||
| 640 | - /// `margin`, `marginTop`, `marginBottom`, `marginRight` — the props the | 649 | + /// `margin` and its four sides — the props the |
| 641 | /// screens use to buy air without a wrapper each time. | 650 | /// screens use to buy air without a wrapper each time. |
| 642 | Widget _margins(core.UiNode n, Widget child) { | 651 | Widget _margins(core.UiNode n, Widget child) { |
| 643 | final all = _d(n.props['margin'], 0); | 652 | final all = _d(n.props['margin'], 0); |
modified
flutter/test/nim_layout_test.dart +2 -0 | @@ -11,6 +11,8 @@ | ||
| 11 | 11 | /// and asserts on widgets passes while the screen is broken. These fail. |
| 12 | 12 | /// |
| 13 | 13 | /// just test layout |
| 14 | +library; | |
| 15 | + | |
| 14 | 16 | import 'package:flutter/material.dart'; |
| 15 | 17 | import 'package:flutter_test/flutter_test.dart'; |
| 16 | 18 | import 'package:frq_core/frq_core.dart' as core; |
| @@ -11,6 +11,8 @@ | |||
| 11 | /// and asserts on widgets passes while the screen is broken. These fail. | 11 | /// and asserts on widgets passes while the screen is broken. These fail. |
| 12 | /// | 12 | /// |
| 13 | /// just test layout | 13 | /// just test layout |
| 14 | +library; | ||
| 15 | + | ||
| 14 | import 'package:flutter/material.dart'; | 16 | import 'package:flutter/material.dart'; |
| 15 | import 'package:flutter_test/flutter_test.dart'; | 17 | import 'package:flutter_test/flutter_test.dart'; |
| 16 | import 'package:frq_core/frq_core.dart' as core; | 18 | import 'package:frq_core/frq_core.dart' as core; |
modified
nim/src/frq/clock.nim +27 -12 | @@ -12,16 +12,13 @@ | ||
| 12 | 12 | ## because the offset moves twice a year, and a backlog read in November |
| 13 | 13 | ## carries messages from August. |
| 14 | 14 | |
| 15 | -import std/[strutils, times] | |
| 15 | +import std/[math, strutils, times] | |
| 16 | 16 | |
| 17 | -func floorDiv*(a, b: int64): int64 = | |
| 18 | - ## Rounds down where `div` rounds toward zero, which for a day number before | |
| 19 | - ## 1970 is a different day. | |
| 20 | - let q = a div b | |
| 21 | - let r = a mod b | |
| 22 | - if r == 0 or (r < 0) == (b < 0): q else: q - 1 | |
| 23 | - | |
| 24 | -func floorMod*(a, b: int64): int64 = a - b * floorDiv(a, b) | |
| 17 | +# `floorDiv` and `floorMod` come from std/math. They round down where `div` | |
| 18 | +# and `mod` round toward zero, which for a day number before 1970 is a | |
| 19 | +# different day — this module had its own copies until it turned out the | |
| 20 | +# stdlib's have exactly these semantics. | |
| 21 | +export floorDiv, floorMod | |
| 25 | 22 | |
| 26 | 23 | func daysFromCivil*(y0, m, d: int64): int64 = |
| 27 | 24 | ## Hinnant's algorithm: a civil date to a day number since the epoch. |
| @@ -45,7 +42,15 @@ func civilFromDays*(days: int64): (int64, int64, int64) = | ||
| 45 | 42 | let m = mp + (if mp < 10: 3 else: -9) |
| 46 | 43 | ((if m <= 2: y + 1 else: y), m, d) |
| 47 | 44 | |
| 48 | -proc nowMs*(): int64 = getTime().toUnix * 1000 + getTime().nanosecond div 1_000_000 | |
| 45 | +proc nowMs*(): int64 = | |
| 46 | + # One `getTime()`, not two: the old form called it twice and could straddle | |
| 47 | + # a second boundary between the halves. | |
| 48 | + let t = getTime() | |
| 49 | + t.toUnix * 1000 + t.nanosecond div 1_000_000 | |
| 50 | + | |
| 51 | +var | |
| 52 | + offsetDay = int64.low ## which UTC day `offsetCache` was computed for | |
| 53 | + offsetCache: int64 | |
| 49 | 54 | |
| 50 | 55 | proc localOffsetSeconds*(epochSecs: int64): int64 = |
| 51 | 56 | ## How far the reader's zone is from UTC at this instant, DST included. |
| @@ -53,8 +58,18 @@ proc localOffsetSeconds*(epochSecs: int64): int64 = | ||
| 53 | 58 | ## Nim has a zone database where the ClojureDart version had to ask the host |
| 54 | 59 | ## through `frq.io` — so this is the one function that got simpler in the |
| 55 | 60 | ## move rather than merely moving. |
| 56 | - let t = fromUnix(epochSecs) | |
| 57 | - t.local.utcOffset.int64 * -1 | |
| 61 | + ## | |
| 62 | + ## Memoised per UTC day, because `.local` is a `localtime_r` and that stats | |
| 63 | + ## /etc/localtime. Every timestamp on screen asks for this three times, and | |
| 64 | + ## the whole tree is rebuilt ten times a second: a busy room was making tens | |
| 65 | + ## of thousands of zone lookups a second to render times that had not | |
| 66 | + ## changed. The offset moves twice a year and never inside a day, so a | |
| 67 | + ## day-granular cache is exact rather than approximate. | |
| 68 | + let day = floorDiv(epochSecs, 86400) | |
| 69 | + if day != offsetDay: | |
| 70 | + offsetDay = day | |
| 71 | + offsetCache = fromUnix(epochSecs).local.utcOffset.int64 * -1 | |
| 72 | + offsetCache | |
| 58 | 73 | |
| 59 | 74 | func parseTimeTag*(tags: string): (int64, bool) = |
| 60 | 75 | ## The `time=` value of an IRCv3 tag string as epoch milliseconds. |
| @@ -12,16 +12,13 @@ | |||
| 12 | ## because the offset moves twice a year, and a backlog read in November | 12 | ## because the offset moves twice a year, and a backlog read in November |
| 13 | ## carries messages from August. | 13 | ## carries messages from August. |
| 14 | 14 | ||
| 15 | -import std/[strutils, times] | 15 | +import std/[math, strutils, times] |
| 16 | 16 | ||
| 17 | -func floorDiv*(a, b: int64): int64 = | 17 | +# `floorDiv` and `floorMod` come from std/math. They round down where `div` |
| 18 | - ## Rounds down where `div` rounds toward zero, which for a day number before | 18 | +# and `mod` round toward zero, which for a day number before 1970 is a |
| 19 | - ## 1970 is a different day. | 19 | +# different day — this module had its own copies until it turned out the |
| 20 | - let q = a div b | 20 | +# stdlib's have exactly these semantics. |
| 21 | - let r = a mod b | 21 | +export floorDiv, floorMod |
| 22 | - if r == 0 or (r < 0) == (b < 0): q else: q - 1 | ||
| 23 | - | ||
| 24 | -func floorMod*(a, b: int64): int64 = a - b * floorDiv(a, b) | ||
| 25 | 22 | ||
| 26 | func daysFromCivil*(y0, m, d: int64): int64 = | 23 | func daysFromCivil*(y0, m, d: int64): int64 = |
| 27 | ## Hinnant's algorithm: a civil date to a day number since the epoch. | 24 | ## Hinnant's algorithm: a civil date to a day number since the epoch. |
| @@ -45,7 +42,15 @@ func civilFromDays*(days: int64): (int64, int64, int64) = | |||
| 45 | let m = mp + (if mp < 10: 3 else: -9) | 42 | let m = mp + (if mp < 10: 3 else: -9) |
| 46 | ((if m <= 2: y + 1 else: y), m, d) | 43 | ((if m <= 2: y + 1 else: y), m, d) |
| 47 | 44 | ||
| 48 | -proc nowMs*(): int64 = getTime().toUnix * 1000 + getTime().nanosecond div 1_000_000 | 45 | +proc nowMs*(): int64 = |
| 46 | + # One `getTime()`, not two: the old form called it twice and could straddle | ||
| 47 | + # a second boundary between the halves. | ||
| 48 | + let t = getTime() | ||
| 49 | + t.toUnix * 1000 + t.nanosecond div 1_000_000 | ||
| 50 | + | ||
| 51 | +var | ||
| 52 | + offsetDay = int64.low ## which UTC day `offsetCache` was computed for | ||
| 53 | + offsetCache: int64 | ||
| 49 | 54 | ||
| 50 | proc localOffsetSeconds*(epochSecs: int64): int64 = | 55 | proc localOffsetSeconds*(epochSecs: int64): int64 = |
| 51 | ## How far the reader's zone is from UTC at this instant, DST included. | 56 | ## How far the reader's zone is from UTC at this instant, DST included. |
| @@ -53,8 +58,18 @@ proc localOffsetSeconds*(epochSecs: int64): int64 = | |||
| 53 | ## Nim has a zone database where the ClojureDart version had to ask the host | 58 | ## Nim has a zone database where the ClojureDart version had to ask the host |
| 54 | ## through `frq.io` — so this is the one function that got simpler in the | 59 | ## through `frq.io` — so this is the one function that got simpler in the |
| 55 | ## move rather than merely moving. | 60 | ## move rather than merely moving. |
| 56 | - let t = fromUnix(epochSecs) | 61 | + ## |
| 57 | - t.local.utcOffset.int64 * -1 | 62 | + ## Memoised per UTC day, because `.local` is a `localtime_r` and that stats |
| 63 | + ## /etc/localtime. Every timestamp on screen asks for this three times, and | ||
| 64 | + ## the whole tree is rebuilt ten times a second: a busy room was making tens | ||
| 65 | + ## of thousands of zone lookups a second to render times that had not | ||
| 66 | + ## changed. The offset moves twice a year and never inside a day, so a | ||
| 67 | + ## day-granular cache is exact rather than approximate. | ||
| 68 | + let day = floorDiv(epochSecs, 86400) | ||
| 69 | + if day != offsetDay: | ||
| 70 | + offsetDay = day | ||
| 71 | + offsetCache = fromUnix(epochSecs).local.utcOffset.int64 * -1 | ||
| 72 | + offsetCache | ||
| 58 | 73 | ||
| 59 | func parseTimeTag*(tags: string): (int64, bool) = | 74 | func parseTimeTag*(tags: string): (int64, bool) = |
| 60 | ## The `time=` value of an IRCv3 tag string as epoch milliseconds. | 75 | ## The `time=` value of an IRCv3 tag string as epoch milliseconds. |
modified
nim/src/frq/conn.nim +1 -1 | @@ -15,7 +15,7 @@ | ||
| 15 | 15 | ## Threading as before: the socket thread shares nothing, and speaks in |
| 16 | 16 | ## channels. See irc.nim's comment for why ORC makes that the sane choice. |
| 17 | 17 | |
| 18 | -import std/[net, os, strutils] | |
| 18 | +import std/net | |
| 19 | 19 | import frq/[trace] |
| 20 | 20 | |
| 21 | 21 | type |
| @@ -15,7 +15,7 @@ | |||
| 15 | ## Threading as before: the socket thread shares nothing, and speaks in | 15 | ## Threading as before: the socket thread shares nothing, and speaks in |
| 16 | ## channels. See irc.nim's comment for why ORC makes that the sane choice. | 16 | ## channels. See irc.nim's comment for why ORC makes that the sane choice. |
| 17 | 17 | ||
| 18 | -import std/[net, os, strutils] | 18 | +import std/net |
| 19 | import frq/[trace] | 19 | import frq/[trace] |
| 20 | 20 | ||
| 21 | type | 21 | type |
modified
nim/src/frq/model.nim +1 -1 | @@ -10,7 +10,7 @@ | ||
| 10 | 10 | ## and a missing key and a null key are the same thing on the other side. An |
| 11 | 11 | ## Option would have to be unwrapped at every boundary anyway. |
| 12 | 12 | |
| 13 | -import std/[options, strutils, tables] | |
| 13 | +import std/[options, strutils] | |
| 14 | 14 | |
| 15 | 15 | type |
| 16 | 16 | Reaction* = object |
| @@ -10,7 +10,7 @@ | |||
| 10 | ## and a missing key and a null key are the same thing on the other side. An | 10 | ## and a missing key and a null key are the same thing on the other side. An |
| 11 | ## Option would have to be unwrapped at every boundary anyway. | 11 | ## Option would have to be unwrapped at every boundary anyway. |
| 12 | 12 | ||
| 13 | -import std/[options, strutils, tables] | 13 | +import std/[options, strutils] |
| 14 | 14 | ||
| 15 | type | 15 | type |
| 16 | Reaction* = object | 16 | Reaction* = object |
modified
nim/src/frq/reducer.nim +8 -24 | @@ -11,10 +11,10 @@ | ||
| 11 | 11 | ## payload: the alternative is a second serialisation to define and version, |
| 12 | 12 | ## for arguments that are always one string. |
| 13 | 13 | |
| 14 | -import std/[json, options, os, sequtils, strutils, tables] | |
| 14 | +import std/[json, options, sequtils, strutils, tables] | |
| 15 | 15 | import std/sets |
| 16 | -import frq/[cells, model, rooms, reactions, edits, trace, ircparse, clock, | |
| 17 | - atproto, handshake] | |
| 16 | +import frq/[cells, model, rooms, reactions, trace, ircparse, clock, | |
| 17 | + atproto, handshake, textruns] | |
| 18 | 18 | import frq/conn as tr |
| 19 | 19 | |
| 20 | 20 | proc split2(id: string): (string, string) = |
| @@ -364,7 +364,11 @@ proc drain*() = | ||
| 364 | 364 | # for the sender: the target is our own nick and is nobody's room. |
| 365 | 365 | let room = if target.startsWith("#"): target else: who |
| 366 | 366 | var m = Message(id: msgid, frm: who, text: p.params[^1], at: at) |
| 367 | - m.imageUrl = "" | |
| 367 | + # The picture link out of the text, which is what draws the inline | |
| 368 | + # preview. This was hardcoded to "" — assigning a field its own | |
| 369 | + # default — so `firstImageUrl` was ported, tested and never called, | |
| 370 | + # and no received message ever showed a preview. | |
| 371 | + m.imageUrl = firstImageUrl(m.text) | |
| 368 | 372 | let (rep, hasRep) = tagValue(p.tags, "+reply") |
| 369 | 373 | if hasRep: m.replyTo = rep |
| 370 | 374 | let (tally, hasTally) = tagValue(p.tags, "+freeq.at/reacts") |
| @@ -428,23 +432,3 @@ proc drain*() = | ||
| 428 | 432 | trace("skip", p.command & " " & $p.params) |
| 429 | 433 | |
| 430 | 434 | |
| 431 | -# -------------------------------------------------------------- autoconnect | |
| 432 | -# | |
| 433 | -# `FRQ_AUTOCONNECT=1` presses Connect on the first render, and `FRQ_NICK` | |
| 434 | -# overrides the nickname. In the same spirit as FRQ_TRACE and for the same | |
| 435 | -# reason: a GUI on Wayland cannot be clicked from a script, so without this the | |
| 436 | -# only way to check that the window gets past the connect screen is to sit in | |
| 437 | -# front of it — which is not a check, and is how a layout error on the chats | |
| 438 | -# screen went unnoticed while the connect screen looked fine. | |
| 439 | - | |
| 440 | -var autoconnectDone = false | |
| 441 | - | |
| 442 | -proc maybeAutoconnect*() = | |
| 443 | - if autoconnectDone: return | |
| 444 | - autoconnectDone = true | |
| 445 | - let want = getEnv("FRQ_AUTOCONNECT") | |
| 446 | - if want.len == 0 or want == "0": return | |
| 447 | - let nick = getEnv("FRQ_NICK") | |
| 448 | - if nick.len > 0: app.formNick = nick | |
| 449 | - trace("auto", "FRQ_AUTOCONNECT set — connecting as " & app.formNick) | |
| 450 | - connectNow() | |
| @@ -11,10 +11,10 @@ | |||
| 11 | ## payload: the alternative is a second serialisation to define and version, | 11 | ## payload: the alternative is a second serialisation to define and version, |
| 12 | ## for arguments that are always one string. | 12 | ## for arguments that are always one string. |
| 13 | 13 | ||
| 14 | -import std/[json, options, os, sequtils, strutils, tables] | 14 | +import std/[json, options, sequtils, strutils, tables] |
| 15 | import std/sets | 15 | import std/sets |
| 16 | -import frq/[cells, model, rooms, reactions, edits, trace, ircparse, clock, | 16 | +import frq/[cells, model, rooms, reactions, trace, ircparse, clock, |
| 17 | - atproto, handshake] | 17 | + atproto, handshake, textruns] |
| 18 | import frq/conn as tr | 18 | import frq/conn as tr |
| 19 | 19 | ||
| 20 | proc split2(id: string): (string, string) = | 20 | proc split2(id: string): (string, string) = |
| @@ -364,7 +364,11 @@ proc drain*() = | |||
| 364 | # for the sender: the target is our own nick and is nobody's room. | 364 | # for the sender: the target is our own nick and is nobody's room. |
| 365 | let room = if target.startsWith("#"): target else: who | 365 | let room = if target.startsWith("#"): target else: who |
| 366 | var m = Message(id: msgid, frm: who, text: p.params[^1], at: at) | 366 | var m = Message(id: msgid, frm: who, text: p.params[^1], at: at) |
| 367 | - m.imageUrl = "" | 367 | + # The picture link out of the text, which is what draws the inline |
| 368 | + # preview. This was hardcoded to "" — assigning a field its own | ||
| 369 | + # default — so `firstImageUrl` was ported, tested and never called, | ||
| 370 | + # and no received message ever showed a preview. | ||
| 371 | + m.imageUrl = firstImageUrl(m.text) | ||
| 368 | let (rep, hasRep) = tagValue(p.tags, "+reply") | 372 | let (rep, hasRep) = tagValue(p.tags, "+reply") |
| 369 | if hasRep: m.replyTo = rep | 373 | if hasRep: m.replyTo = rep |
| 370 | let (tally, hasTally) = tagValue(p.tags, "+freeq.at/reacts") | 374 | let (tally, hasTally) = tagValue(p.tags, "+freeq.at/reacts") |
| @@ -428,23 +432,3 @@ proc drain*() = | |||
| 428 | trace("skip", p.command & " " & $p.params) | 432 | trace("skip", p.command & " " & $p.params) |
| 429 | 433 | ||
| 430 | 434 | ||
| 431 | -# -------------------------------------------------------------- autoconnect | ||
| 432 | -# | ||
| 433 | -# `FRQ_AUTOCONNECT=1` presses Connect on the first render, and `FRQ_NICK` | ||
| 434 | -# overrides the nickname. In the same spirit as FRQ_TRACE and for the same | ||
| 435 | -# reason: a GUI on Wayland cannot be clicked from a script, so without this the | ||
| 436 | -# only way to check that the window gets past the connect screen is to sit in | ||
| 437 | -# front of it — which is not a check, and is how a layout error on the chats | ||
| 438 | -# screen went unnoticed while the connect screen looked fine. | ||
| 439 | - | ||
| 440 | -var autoconnectDone = false | ||
| 441 | - | ||
| 442 | -proc maybeAutoconnect*() = | ||
| 443 | - if autoconnectDone: return | ||
| 444 | - autoconnectDone = true | ||
| 445 | - let want = getEnv("FRQ_AUTOCONNECT") | ||
| 446 | - if want.len == 0 or want == "0": return | ||
| 447 | - let nick = getEnv("FRQ_NICK") | ||
| 448 | - if nick.len > 0: app.formNick = nick | ||
| 449 | - trace("auto", "FRQ_AUTOCONNECT set — connecting as " & app.formNick) | ||
| 450 | - connectNow() | ||
modified
nim/src/frq/screens/chat.nim +30 -33 | @@ -12,8 +12,7 @@ | ||
| 12 | 12 | ## dead buttons in ClojureDart and dead buttons here. When Flutter's camera |
| 13 | 13 | ## and audio plugins arrive this is the file they come back to. |
| 14 | 14 | |
| 15 | -import std/[algorithm, json, strutils, tables] | |
| 16 | -from std/unicode import runeLen, runeSubStr | |
| 15 | +import std/[algorithm, json, strutils] | |
| 17 | 16 | import std/options |
| 18 | 17 | import frq/[ui, cells, model, clock, reactions, textruns] |
| 19 | 18 | from frq/screens/connect import errorNote |
| @@ -24,23 +23,7 @@ const | ||
| 24 | 23 | chipGap = 4 |
| 25 | 24 | overviewLines* = 8 |
| 26 | 25 | |
| 27 | -func summarise*(text: string, n: int): string = | |
| 28 | - ## What a reply chip quotes back. One line, cut to fit. | |
| 29 | - var line = newStringOfCap(text.len) | |
| 30 | - var inSpace = false | |
| 31 | - for c in text: | |
| 32 | - if c in {' ', '\t', '\n', '\r'}: | |
| 33 | - if not inSpace: line.add ' ' | |
| 34 | - inSpace = true | |
| 35 | - else: | |
| 36 | - line.add c | |
| 37 | - inSpace = false | |
| 38 | - line = line.strip() | |
| 39 | - # Runes, not bytes: a byte slice lands inside a multi-byte character and | |
| 40 | - # makes mojibake where an ellipsis was wanted. | |
| 41 | - if line.runeLen > n: line.runeSubStr(0, n - 1) & "…" else: line | |
| 42 | - | |
| 43 | -func actionChips(room: string, m: Message, mine: bool): Node = | |
| 26 | +func actionChips(m: Message, mine: bool): Node = | |
| 44 | 27 | ## Answering and reacting, on the sender's row above the message. |
| 45 | 28 | ## |
| 46 | 29 | ## Both are things done *to* a message rather than parts of it, so they ride |
| @@ -90,13 +73,14 @@ func runNodes(m: Message): Node = | ||
| 90 | 73 | ## line of its own, and a plain wrapping row measures each label against the |
| 91 | 74 | ## row's width rather than the column's, which is what drags long URLs off |
| 92 | 75 | ## the left edge. |
| 93 | - result = n("hbox", %*{"key": "runs", "wrap": true, "inline": true}) | |
| 76 | + result = paragraph() | |
| 77 | + result.props["key"] = %"runs" | |
| 94 | 78 | for r in textRuns(m.text): |
| 95 | 79 | case r.kind |
| 96 | 80 | of rkText: result.children.add text(r.value) |
| 97 | 81 | of rkLink: result.children.add link(r.value, r.value) |
| 98 | 82 | |
| 99 | -proc messageBody(s: State, m: Message, highlit: bool): Node = | |
| 83 | +proc messageBody(s: State, room: Room, m: Message, highlit: bool): Node = | |
| 100 | 84 | ## A message without its face: the sender's line, the words, and what hangs |
| 101 | 85 | ## under them. |
| 102 | 86 | var who = vbox(%*{"key": "who"}) |
| @@ -112,7 +96,7 @@ proc messageBody(s: State, m: Message, highlit: bool): Node = | ||
| 112 | 96 | if m.edited: |
| 113 | 97 | row.children.add dimLabel("(edited)") |
| 114 | 98 | if m.id.len > 0: |
| 115 | - row.children.add actionChips(s.current, m, m.frm == s.formNick) | |
| 99 | + row.children.add actionChips(m, m.frm == s.formNick) | |
| 116 | 100 | else: |
| 117 | 101 | # A spacer where the chips would be, so a line with no msgid is a row of |
| 118 | 102 | # the same shape rather than a row with a hole in it. |
| @@ -123,7 +107,7 @@ proc messageBody(s: State, m: Message, highlit: bool): Node = | ||
| 123 | 107 | # error note is: a child that comes and goes renumbers the row. |
| 124 | 108 | var chip = vbox(%*{"key": "reply-chip"}) |
| 125 | 109 | if m.replyTo.len > 0: |
| 126 | - let target = s.currentRoom.messageById(m.replyTo) | |
| 110 | + let target = room.messageById(m.replyTo) | |
| 127 | 111 | if target.isSome: |
| 128 | 112 | chip.children.add replyChip(target.get) |
| 129 | 113 | else: |
| @@ -150,7 +134,7 @@ proc messageBody(s: State, m: Message, highlit: bool): Node = | ||
| 150 | 134 | "spacing": 2, "margin": 0}, |
| 151 | 135 | @[who, body, images, pills]) |
| 152 | 136 | |
| 153 | -proc messageRow(s: State, i: int, m: Message): Node = | |
| 137 | +proc messageRow(s: State, room: Room, i: int, m: Message): Node = | |
| 154 | 138 | ## One message: who said it, when, what you can do to it, and the words. |
| 155 | 139 | ## |
| 156 | 140 | ## Every line names its sender, rather than the first of a run only. A run |
| @@ -162,24 +146,33 @@ proc messageRow(s: State, i: int, m: Message): Node = | ||
| 162 | 146 | n("vbox", %*{"key": $i, "spacing": 2, "margin": 0, "marginRight": 10, |
| 163 | 147 | "marginTop": 10, |
| 164 | 148 | "scrollHere": rid.len > 0 and rid == s.jumpTo}, |
| 165 | - @[messageBody(s, m, highlit)]) | |
| 149 | + @[messageBody(s, room, m, highlit)]) | |
| 166 | 150 | |
| 167 | 151 | func daySeparator(key, label0: string): Node = |
| 168 | 152 | n("hbox", %*{"key": key, "spacing": 8}, @[separator(), dimLabel(label0)]) |
| 169 | 153 | |
| 170 | -proc messageRows*(s: State, messages: seq[Message]): seq[Node] = | |
| 154 | +proc messageRows*(s: State, room: Room, messages: seq[Message]): seq[Node] = | |
| 171 | 155 | ## The messages, with a heading wherever the day changes. |
| 172 | 156 | ## |
| 157 | + ## `room` is passed down rather than read from `s` where it is wanted, and | |
| 158 | + ## that is not tidiness: `State.currentRoom` returns a Room **by value**, and | |
| 159 | + ## a Room owns its whole backlog, so every call deep-copies every message in | |
| 160 | + ## it. Resolving a reply that way — once per replying line, per render — | |
| 161 | + ## was 100 × 500 message copies a frame in a busy room, ten times a second. | |
| 162 | + ## | |
| 173 | 163 | ## A backlog can reach back weeks, and `11:04 AM` says nothing about which |
| 174 | 164 | ## day it was. The heading is what makes the time above it mean something. |
| 165 | + # `prevDay` is carried rather than recomputed: asking `day()` for the | |
| 166 | + # previous message repeated the work the previous iteration had already | |
| 167 | + # done, doubling the zone lookups for the whole backlog. | |
| 168 | + var prevDay = "" | |
| 175 | 169 | for i, m in messages: |
| 176 | 170 | if m.at > 0: |
| 177 | 171 | let d = day(m.at) |
| 178 | - let prevDay = if i > 0 and messages[i - 1].at > 0: day(messages[i - 1].at) | |
| 179 | - else: "" | |
| 180 | 172 | if d != prevDay: |
| 181 | 173 | result.add daySeparator("day-" & $i, dayLabel(m.at)) |
| 182 | - result.add messageRow(s, i, m) | |
| 174 | + prevDay = d | |
| 175 | + result.add messageRow(s, room, i, m) | |
| 183 | 176 | |
| 184 | 177 | proc visible(s: State, messages: seq[Message]): seq[Message] = |
| 185 | 178 | ## The lines this reader wants to see. Comings and goings are the room |
| @@ -237,7 +230,7 @@ proc chatScreen*(s: State, connected: bool): Node = | ||
| 237 | 230 | |
| 238 | 231 | # The backlog. Not a page — a page scrolls everything, which would carry the |
| 239 | 232 | # compose bar off the bottom with the messages. |
| 240 | - var messages = vbox(%*{"key": "messages", "fillHeight": not narrowPeople}) | |
| 233 | + var messages = vbox(%*{"key": "messages", "expand": not narrowPeople}) | |
| 241 | 234 | if not narrowPeople: |
| 242 | 235 | var sc = scroll(%*{"scrollKey": "messages-" & room.name, |
| 243 | 236 | "orientation": "vertical", |
| @@ -245,7 +238,7 @@ proc chatScreen*(s: State, connected: bool): Node = | ||
| 245 | 238 | "scrollToBottom": s.jumpTick}) |
| 246 | 239 | let shown = visible(s, room.messages) |
| 247 | 240 | if shown.len > 0: |
| 248 | - for node in messageRows(s, shown): | |
| 241 | + for node in messageRows(s, room, shown): | |
| 249 | 242 | sc.children.add node |
| 250 | 243 | else: |
| 251 | 244 | sc.children.add dimLabel("Nothing here yet.") |
| @@ -299,10 +292,14 @@ proc chatScreen*(s: State, connected: bool): Node = | ||
| 299 | 292 | width = 260, onSubmit = "send"), |
| 300 | 293 | button("Send", "send", "primary")) |
| 301 | 294 | |
| 302 | - vbox(%*{"spacing": 8, "margin": 12, "fillHeight": true}, | |
| 295 | + vbox(%*{"spacing": 8, "margin": 12, "expand": true}, | |
| 303 | 296 | headRow, |
| 304 | 297 | errorNote(s), |
| 305 | - hbox(%*{"spacing": 8, "wrap": false}, messages, peoplePane), | |
| 298 | + # `expand` on the row itself: it is the thing that takes the column's | |
| 299 | + # remaining height. The renderer used to infer that by looking at this | |
| 300 | + # row's children, which is the prop being on the wrong node. | |
| 301 | + n("hbox", %*{"spacing": 8, "wrap": false, "expand": true}, | |
| 302 | + @[messages, peoplePane]), | |
| 306 | 303 | jump, |
| 307 | 304 | banners, |
| 308 | 305 | separator(), |
| @@ -12,8 +12,7 @@ | |||
| 12 | ## dead buttons in ClojureDart and dead buttons here. When Flutter's camera | 12 | ## dead buttons in ClojureDart and dead buttons here. When Flutter's camera |
| 13 | ## and audio plugins arrive this is the file they come back to. | 13 | ## and audio plugins arrive this is the file they come back to. |
| 14 | 14 | ||
| 15 | -import std/[algorithm, json, strutils, tables] | 15 | +import std/[algorithm, json, strutils] |
| 16 | -from std/unicode import runeLen, runeSubStr | ||
| 17 | import std/options | 16 | import std/options |
| 18 | import frq/[ui, cells, model, clock, reactions, textruns] | 17 | import frq/[ui, cells, model, clock, reactions, textruns] |
| 19 | from frq/screens/connect import errorNote | 18 | from frq/screens/connect import errorNote |
| @@ -24,23 +23,7 @@ const | |||
| 24 | chipGap = 4 | 23 | chipGap = 4 |
| 25 | overviewLines* = 8 | 24 | overviewLines* = 8 |
| 26 | 25 | ||
| 27 | -func summarise*(text: string, n: int): string = | 26 | +func actionChips(m: Message, mine: bool): Node = |
| 28 | - ## What a reply chip quotes back. One line, cut to fit. | ||
| 29 | - var line = newStringOfCap(text.len) | ||
| 30 | - var inSpace = false | ||
| 31 | - for c in text: | ||
| 32 | - if c in {' ', '\t', '\n', '\r'}: | ||
| 33 | - if not inSpace: line.add ' ' | ||
| 34 | - inSpace = true | ||
| 35 | - else: | ||
| 36 | - line.add c | ||
| 37 | - inSpace = false | ||
| 38 | - line = line.strip() | ||
| 39 | - # Runes, not bytes: a byte slice lands inside a multi-byte character and | ||
| 40 | - # makes mojibake where an ellipsis was wanted. | ||
| 41 | - if line.runeLen > n: line.runeSubStr(0, n - 1) & "…" else: line | ||
| 42 | - | ||
| 43 | -func actionChips(room: string, m: Message, mine: bool): Node = | ||
| 44 | ## Answering and reacting, on the sender's row above the message. | 27 | ## Answering and reacting, on the sender's row above the message. |
| 45 | ## | 28 | ## |
| 46 | ## Both are things done *to* a message rather than parts of it, so they ride | 29 | ## Both are things done *to* a message rather than parts of it, so they ride |
| @@ -90,13 +73,14 @@ func runNodes(m: Message): Node = | |||
| 90 | ## line of its own, and a plain wrapping row measures each label against the | 73 | ## line of its own, and a plain wrapping row measures each label against the |
| 91 | ## row's width rather than the column's, which is what drags long URLs off | 74 | ## row's width rather than the column's, which is what drags long URLs off |
| 92 | ## the left edge. | 75 | ## the left edge. |
| 93 | - result = n("hbox", %*{"key": "runs", "wrap": true, "inline": true}) | 76 | + result = paragraph() |
| 77 | + result.props["key"] = %"runs" | ||
| 94 | for r in textRuns(m.text): | 78 | for r in textRuns(m.text): |
| 95 | case r.kind | 79 | case r.kind |
| 96 | of rkText: result.children.add text(r.value) | 80 | of rkText: result.children.add text(r.value) |
| 97 | of rkLink: result.children.add link(r.value, r.value) | 81 | of rkLink: result.children.add link(r.value, r.value) |
| 98 | 82 | ||
| 99 | -proc messageBody(s: State, m: Message, highlit: bool): Node = | 83 | +proc messageBody(s: State, room: Room, m: Message, highlit: bool): Node = |
| 100 | ## A message without its face: the sender's line, the words, and what hangs | 84 | ## A message without its face: the sender's line, the words, and what hangs |
| 101 | ## under them. | 85 | ## under them. |
| 102 | var who = vbox(%*{"key": "who"}) | 86 | var who = vbox(%*{"key": "who"}) |
| @@ -112,7 +96,7 @@ proc messageBody(s: State, m: Message, highlit: bool): Node = | |||
| 112 | if m.edited: | 96 | if m.edited: |
| 113 | row.children.add dimLabel("(edited)") | 97 | row.children.add dimLabel("(edited)") |
| 114 | if m.id.len > 0: | 98 | if m.id.len > 0: |
| 115 | - row.children.add actionChips(s.current, m, m.frm == s.formNick) | 99 | + row.children.add actionChips(m, m.frm == s.formNick) |
| 116 | else: | 100 | else: |
| 117 | # A spacer where the chips would be, so a line with no msgid is a row of | 101 | # A spacer where the chips would be, so a line with no msgid is a row of |
| 118 | # the same shape rather than a row with a hole in it. | 102 | # the same shape rather than a row with a hole in it. |
| @@ -123,7 +107,7 @@ proc messageBody(s: State, m: Message, highlit: bool): Node = | |||
| 123 | # error note is: a child that comes and goes renumbers the row. | 107 | # error note is: a child that comes and goes renumbers the row. |
| 124 | var chip = vbox(%*{"key": "reply-chip"}) | 108 | var chip = vbox(%*{"key": "reply-chip"}) |
| 125 | if m.replyTo.len > 0: | 109 | if m.replyTo.len > 0: |
| 126 | - let target = s.currentRoom.messageById(m.replyTo) | 110 | + let target = room.messageById(m.replyTo) |
| 127 | if target.isSome: | 111 | if target.isSome: |
| 128 | chip.children.add replyChip(target.get) | 112 | chip.children.add replyChip(target.get) |
| 129 | else: | 113 | else: |
| @@ -150,7 +134,7 @@ proc messageBody(s: State, m: Message, highlit: bool): Node = | |||
| 150 | "spacing": 2, "margin": 0}, | 134 | "spacing": 2, "margin": 0}, |
| 151 | @[who, body, images, pills]) | 135 | @[who, body, images, pills]) |
| 152 | 136 | ||
| 153 | -proc messageRow(s: State, i: int, m: Message): Node = | 137 | +proc messageRow(s: State, room: Room, i: int, m: Message): Node = |
| 154 | ## One message: who said it, when, what you can do to it, and the words. | 138 | ## One message: who said it, when, what you can do to it, and the words. |
| 155 | ## | 139 | ## |
| 156 | ## Every line names its sender, rather than the first of a run only. A run | 140 | ## Every line names its sender, rather than the first of a run only. A run |
| @@ -162,24 +146,33 @@ proc messageRow(s: State, i: int, m: Message): Node = | |||
| 162 | n("vbox", %*{"key": $i, "spacing": 2, "margin": 0, "marginRight": 10, | 146 | n("vbox", %*{"key": $i, "spacing": 2, "margin": 0, "marginRight": 10, |
| 163 | "marginTop": 10, | 147 | "marginTop": 10, |
| 164 | "scrollHere": rid.len > 0 and rid == s.jumpTo}, | 148 | "scrollHere": rid.len > 0 and rid == s.jumpTo}, |
| 165 | - @[messageBody(s, m, highlit)]) | 149 | + @[messageBody(s, room, m, highlit)]) |
| 166 | 150 | ||
| 167 | func daySeparator(key, label0: string): Node = | 151 | func daySeparator(key, label0: string): Node = |
| 168 | n("hbox", %*{"key": key, "spacing": 8}, @[separator(), dimLabel(label0)]) | 152 | n("hbox", %*{"key": key, "spacing": 8}, @[separator(), dimLabel(label0)]) |
| 169 | 153 | ||
| 170 | -proc messageRows*(s: State, messages: seq[Message]): seq[Node] = | 154 | +proc messageRows*(s: State, room: Room, messages: seq[Message]): seq[Node] = |
| 171 | ## The messages, with a heading wherever the day changes. | 155 | ## The messages, with a heading wherever the day changes. |
| 172 | ## | 156 | ## |
| 157 | + ## `room` is passed down rather than read from `s` where it is wanted, and | ||
| 158 | + ## that is not tidiness: `State.currentRoom` returns a Room **by value**, and | ||
| 159 | + ## a Room owns its whole backlog, so every call deep-copies every message in | ||
| 160 | + ## it. Resolving a reply that way — once per replying line, per render — | ||
| 161 | + ## was 100 × 500 message copies a frame in a busy room, ten times a second. | ||
| 162 | + ## | ||
| 173 | ## A backlog can reach back weeks, and `11:04 AM` says nothing about which | 163 | ## A backlog can reach back weeks, and `11:04 AM` says nothing about which |
| 174 | ## day it was. The heading is what makes the time above it mean something. | 164 | ## day it was. The heading is what makes the time above it mean something. |
| 165 | + # `prevDay` is carried rather than recomputed: asking `day()` for the | ||
| 166 | + # previous message repeated the work the previous iteration had already | ||
| 167 | + # done, doubling the zone lookups for the whole backlog. | ||
| 168 | + var prevDay = "" | ||
| 175 | for i, m in messages: | 169 | for i, m in messages: |
| 176 | if m.at > 0: | 170 | if m.at > 0: |
| 177 | let d = day(m.at) | 171 | let d = day(m.at) |
| 178 | - let prevDay = if i > 0 and messages[i - 1].at > 0: day(messages[i - 1].at) | ||
| 179 | - else: "" | ||
| 180 | if d != prevDay: | 172 | if d != prevDay: |
| 181 | result.add daySeparator("day-" & $i, dayLabel(m.at)) | 173 | result.add daySeparator("day-" & $i, dayLabel(m.at)) |
| 182 | - result.add messageRow(s, i, m) | 174 | + prevDay = d |
| 175 | + result.add messageRow(s, room, i, m) | ||
| 183 | 176 | ||
| 184 | proc visible(s: State, messages: seq[Message]): seq[Message] = | 177 | proc visible(s: State, messages: seq[Message]): seq[Message] = |
| 185 | ## The lines this reader wants to see. Comings and goings are the room | 178 | ## The lines this reader wants to see. Comings and goings are the room |
| @@ -237,7 +230,7 @@ proc chatScreen*(s: State, connected: bool): Node = | |||
| 237 | 230 | ||
| 238 | # The backlog. Not a page — a page scrolls everything, which would carry the | 231 | # The backlog. Not a page — a page scrolls everything, which would carry the |
| 239 | # compose bar off the bottom with the messages. | 232 | # compose bar off the bottom with the messages. |
| 240 | - var messages = vbox(%*{"key": "messages", "fillHeight": not narrowPeople}) | 233 | + var messages = vbox(%*{"key": "messages", "expand": not narrowPeople}) |
| 241 | if not narrowPeople: | 234 | if not narrowPeople: |
| 242 | var sc = scroll(%*{"scrollKey": "messages-" & room.name, | 235 | var sc = scroll(%*{"scrollKey": "messages-" & room.name, |
| 243 | "orientation": "vertical", | 236 | "orientation": "vertical", |
| @@ -245,7 +238,7 @@ proc chatScreen*(s: State, connected: bool): Node = | |||
| 245 | "scrollToBottom": s.jumpTick}) | 238 | "scrollToBottom": s.jumpTick}) |
| 246 | let shown = visible(s, room.messages) | 239 | let shown = visible(s, room.messages) |
| 247 | if shown.len > 0: | 240 | if shown.len > 0: |
| 248 | - for node in messageRows(s, shown): | 241 | + for node in messageRows(s, room, shown): |
| 249 | sc.children.add node | 242 | sc.children.add node |
| 250 | else: | 243 | else: |
| 251 | sc.children.add dimLabel("Nothing here yet.") | 244 | sc.children.add dimLabel("Nothing here yet.") |
| @@ -299,10 +292,14 @@ proc chatScreen*(s: State, connected: bool): Node = | |||
| 299 | width = 260, onSubmit = "send"), | 292 | width = 260, onSubmit = "send"), |
| 300 | button("Send", "send", "primary")) | 293 | button("Send", "send", "primary")) |
| 301 | 294 | ||
| 302 | - vbox(%*{"spacing": 8, "margin": 12, "fillHeight": true}, | 295 | + vbox(%*{"spacing": 8, "margin": 12, "expand": true}, |
| 303 | headRow, | 296 | headRow, |
| 304 | errorNote(s), | 297 | errorNote(s), |
| 305 | - hbox(%*{"spacing": 8, "wrap": false}, messages, peoplePane), | 298 | + # `expand` on the row itself: it is the thing that takes the column's |
| 299 | + # remaining height. The renderer used to infer that by looking at this | ||
| 300 | + # row's children, which is the prop being on the wrong node. | ||
| 301 | + n("hbox", %*{"spacing": 8, "wrap": false, "expand": true}, | ||
| 302 | + @[messages, peoplePane]), | ||
| 306 | jump, | 303 | jump, |
| 307 | banners, | 304 | banners, |
| 308 | separator(), | 305 | separator(), |
modified
nim/src/frq/screens/chats.nim +10 -26 | @@ -4,12 +4,8 @@ | ||
| 4 | 4 | ## dropped rather than translated — there is no terminal frontend any more, so |
| 5 | 5 | ## `conversation-row`'s two layouts collapse to the one a window uses. |
| 6 | 6 | |
| 7 | -import std/[json, strutils, tables] | |
| 8 | -# `runeLen` and `runeSubStr` only: a plain `import std/unicode` brings a | |
| 9 | -# `title` that is ambiguous against `ui.title`, which is the one this file | |
| 10 | -# means every time it says it. | |
| 11 | -from std/unicode import runeLen, runeSubStr | |
| 12 | -import frq/[ui, cells, model, rooms] | |
| 7 | +import std/[json, strutils] | |
| 8 | +import frq/[ui, cells, model, rooms, textruns] | |
| 13 | 9 | import frq/screens/[frame, connect] |
| 14 | 10 | |
| 15 | 11 | const listGutter = 16 |
| @@ -17,23 +13,12 @@ const listGutter = 16 | ||
| 17 | 13 | ## character of a preview is behind the thumb. |
| 18 | 14 | |
| 19 | 15 | func previewLine*(text: string): string = |
| 20 | - ## The last line of a conversation, as one line. | |
| 16 | + ## The last line of a conversation, as one line, cut to fit a card. | |
| 21 | 17 | ## |
| 22 | - ## A pasted shell script or a long link is a card's worth of text otherwise, | |
| 23 | - ## and the cards stop reading as a list of rooms. | |
| 24 | - var line = newStringOfCap(text.len) | |
| 25 | - var inSpace = false | |
| 26 | - for c in text: | |
| 27 | - if c in {' ', '\t', '\n', '\r'}: | |
| 28 | - if not inSpace: line.add ' ' | |
| 29 | - inSpace = true | |
| 30 | - else: | |
| 31 | - line.add c | |
| 32 | - inSpace = false | |
| 33 | - # Runes and not bytes. Clojure's `count` and `subs` are characters, and a | |
| 34 | - # byte slice at 59 can land in the middle of one — which for a preview full | |
| 35 | - # of emoji is a truncation that produces mojibake rather than an ellipsis. | |
| 36 | - if line.runeLen > 60: line.runeSubStr(0, 59) & "…" else: line | |
| 18 | + ## 60 characters is the card's width; the truncating itself is | |
| 19 | + ## `textruns.summarise`, which the reply chip in the chat screen uses for | |
| 20 | + ## the same job. | |
| 21 | + summarise(text, 60) | |
| 37 | 22 | |
| 38 | 23 | func conversationRow(r: Room): Node = |
| 39 | 24 | var badges = vbox(%*{"key": "badges"}) |
| @@ -88,11 +73,10 @@ func chatsScreen*(s: State, connected: bool): Node = | ||
| 88 | 73 | body = n("vbox", %*{"marginRight": listGutter}, @[ |
| 89 | 74 | card(dimLabel("No conversations yet — join a channel."))]) |
| 90 | 75 | |
| 91 | - vbox(%*{"spacing": 8, "margin": 12, "fillHeight": true}, | |
| 76 | + vbox(%*{"spacing": 8, "margin": 12, "expand": true}, | |
| 92 | 77 | head, |
| 93 | - vbox(%*{"key": "list", "fillHeight": true}, | |
| 94 | - scroll(%*{"scrollKey": "chats-list", "orientation": "vertical", | |
| 95 | - "reserve": belowList(s)}, body)), | |
| 78 | + vbox(%*{"key": "list", "expand": true}, | |
| 79 | + scroll(%*{"scrollKey": "chats-list", "orientation": "vertical"}, body)), | |
| 96 | 80 | vbox(%*{"key": "foot", "spacing": 8}, |
| 97 | 81 | separator(), |
| 98 | 82 | tabBar(s))) |
| @@ -4,12 +4,8 @@ | |||
| 4 | ## dropped rather than translated — there is no terminal frontend any more, so | 4 | ## dropped rather than translated — there is no terminal frontend any more, so |
| 5 | ## `conversation-row`'s two layouts collapse to the one a window uses. | 5 | ## `conversation-row`'s two layouts collapse to the one a window uses. |
| 6 | 6 | ||
| 7 | -import std/[json, strutils, tables] | 7 | +import std/[json, strutils] |
| 8 | -# `runeLen` and `runeSubStr` only: a plain `import std/unicode` brings a | 8 | +import frq/[ui, cells, model, rooms, textruns] |
| 9 | -# `title` that is ambiguous against `ui.title`, which is the one this file | ||
| 10 | -# means every time it says it. | ||
| 11 | -from std/unicode import runeLen, runeSubStr | ||
| 12 | -import frq/[ui, cells, model, rooms] | ||
| 13 | import frq/screens/[frame, connect] | 9 | import frq/screens/[frame, connect] |
| 14 | 10 | ||
| 15 | const listGutter = 16 | 11 | const listGutter = 16 |
| @@ -17,23 +13,12 @@ const listGutter = 16 | |||
| 17 | ## character of a preview is behind the thumb. | 13 | ## character of a preview is behind the thumb. |
| 18 | 14 | ||
| 19 | func previewLine*(text: string): string = | 15 | func previewLine*(text: string): string = |
| 20 | - ## The last line of a conversation, as one line. | 16 | + ## The last line of a conversation, as one line, cut to fit a card. |
| 21 | ## | 17 | ## |
| 22 | - ## A pasted shell script or a long link is a card's worth of text otherwise, | 18 | + ## 60 characters is the card's width; the truncating itself is |
| 23 | - ## and the cards stop reading as a list of rooms. | 19 | + ## `textruns.summarise`, which the reply chip in the chat screen uses for |
| 24 | - var line = newStringOfCap(text.len) | 20 | + ## the same job. |
| 25 | - var inSpace = false | 21 | + summarise(text, 60) |
| 26 | - for c in text: | ||
| 27 | - if c in {' ', '\t', '\n', '\r'}: | ||
| 28 | - if not inSpace: line.add ' ' | ||
| 29 | - inSpace = true | ||
| 30 | - else: | ||
| 31 | - line.add c | ||
| 32 | - inSpace = false | ||
| 33 | - # Runes and not bytes. Clojure's `count` and `subs` are characters, and a | ||
| 34 | - # byte slice at 59 can land in the middle of one — which for a preview full | ||
| 35 | - # of emoji is a truncation that produces mojibake rather than an ellipsis. | ||
| 36 | - if line.runeLen > 60: line.runeSubStr(0, 59) & "…" else: line | ||
| 37 | 22 | ||
| 38 | func conversationRow(r: Room): Node = | 23 | func conversationRow(r: Room): Node = |
| 39 | var badges = vbox(%*{"key": "badges"}) | 24 | var badges = vbox(%*{"key": "badges"}) |
| @@ -88,11 +73,10 @@ func chatsScreen*(s: State, connected: bool): Node = | |||
| 88 | body = n("vbox", %*{"marginRight": listGutter}, @[ | 73 | body = n("vbox", %*{"marginRight": listGutter}, @[ |
| 89 | card(dimLabel("No conversations yet — join a channel."))]) | 74 | card(dimLabel("No conversations yet — join a channel."))]) |
| 90 | 75 | ||
| 91 | - vbox(%*{"spacing": 8, "margin": 12, "fillHeight": true}, | 76 | + vbox(%*{"spacing": 8, "margin": 12, "expand": true}, |
| 92 | head, | 77 | head, |
| 93 | - vbox(%*{"key": "list", "fillHeight": true}, | 78 | + vbox(%*{"key": "list", "expand": true}, |
| 94 | - scroll(%*{"scrollKey": "chats-list", "orientation": "vertical", | 79 | + scroll(%*{"scrollKey": "chats-list", "orientation": "vertical"}, body)), |
| 95 | - "reserve": belowList(s)}, body)), | ||
| 96 | vbox(%*{"key": "foot", "spacing": 8}, | 80 | vbox(%*{"key": "foot", "spacing": 8}, |
| 97 | separator(), | 81 | separator(), |
| 98 | tabBar(s))) | 82 | tabBar(s))) |
modified
nim/src/frq/screens/connect.nim +3 -1 | @@ -12,7 +12,9 @@ import std/json | ||
| 12 | 12 | import frq/[ui, cells] |
| 13 | 13 | |
| 14 | 14 | const transportNote* = |
| 15 | - "TLS comes from dart:io, so :6697 works here; untick it for a plain :6667 listener." | |
| 15 | + # Was "TLS comes from dart:io", which stopped being true when the socket | |
| 16 | + # moved to Nim's std/net — and this is on the screen, not in a comment. | |
| 17 | + "TLS on :6697; untick it for a server's plain :6667 listener." | |
| 16 | 18 | |
| 17 | 19 | func errorNote*(s: State): Node = |
| 18 | 20 | ## Always a node, never nothing. |
| @@ -12,7 +12,9 @@ import std/json | |||
| 12 | import frq/[ui, cells] | 12 | import frq/[ui, cells] |
| 13 | 13 | ||
| 14 | const transportNote* = | 14 | const transportNote* = |
| 15 | - "TLS comes from dart:io, so :6697 works here; untick it for a plain :6667 listener." | 15 | + # Was "TLS comes from dart:io", which stopped being true when the socket |
| 16 | + # moved to Nim's std/net — and this is on the screen, not in a comment. | ||
| 17 | + "TLS on :6697; untick it for a server's plain :6667 listener." | ||
| 16 | 18 | ||
| 17 | func errorNote*(s: State): Node = | 19 | func errorNote*(s: State): Node = |
| 18 | ## Always a node, never nothing. | 20 | ## Always a node, never nothing. |
modified
nim/src/frq/screens/frame.nim +3 -12 | @@ -16,15 +16,6 @@ func tabBar*(s: State): Node = | ||
| 16 | 16 | button("Settings", "screen.settings", |
| 17 | 17 | if s.screen == scSettings: "primary" else: "default")) |
| 18 | 18 | |
| 19 | -func belowList*(s: State): int = | |
| 20 | - ## How many points the strip under the list needs, so the scroll view knows | |
| 21 | - ## what to reserve. A separator and a row of tabs. | |
| 22 | - ## | |
| 23 | - ## Counted in chrome rows rather than points in the Clojure, because a | |
| 24 | - ## terminal's row is one cell and a window's is 34. There is no terminal any | |
| 25 | - ## more, so this is the window's number. | |
| 26 | - 46 | |
| 27 | - | |
| 28 | 19 | func tabScreen*(s: State, title0, scrollKey: string, |
| 29 | 20 | body: varargs[Node]): Node = |
| 30 | 21 | ## One of the three screens the tab bar moves between: the title at the top, |
| @@ -34,14 +25,14 @@ func tabScreen*(s: State, title0, scrollKey: string, | ||
| 34 | 25 | ## middle. As pages they were centred columns of their own widths with the |
| 35 | 26 | ## tabs wherever the content happened to end, and every switch resized the |
| 36 | 27 | ## screen under the pointer. |
| 37 | - var list = vbox(%*{"key": "list", "fillHeight": true}) | |
| 28 | + var list = vbox(%*{"key": "list", "expand": true}) | |
| 38 | 29 | var sc = scroll(%*{"scrollKey": scrollKey, "orientation": "vertical", |
| 39 | - "reserve": belowList(s), "spacing": 8}) | |
| 30 | + "spacing": 8}) | |
| 40 | 31 | for b in body: |
| 41 | 32 | if not b.isNil: sc.children.add b |
| 42 | 33 | list.children.add sc |
| 43 | 34 | |
| 44 | - vbox(%*{"spacing": 8, "margin": 12, "fillHeight": true}, | |
| 35 | + vbox(%*{"spacing": 8, "margin": 12, "expand": true}, | |
| 45 | 36 | title(title0), |
| 46 | 37 | list, |
| 47 | 38 | vbox(%*{"key": "foot", "spacing": 8}, |
| @@ -16,15 +16,6 @@ func tabBar*(s: State): Node = | |||
| 16 | button("Settings", "screen.settings", | 16 | button("Settings", "screen.settings", |
| 17 | if s.screen == scSettings: "primary" else: "default")) | 17 | if s.screen == scSettings: "primary" else: "default")) |
| 18 | 18 | ||
| 19 | -func belowList*(s: State): int = | ||
| 20 | - ## How many points the strip under the list needs, so the scroll view knows | ||
| 21 | - ## what to reserve. A separator and a row of tabs. | ||
| 22 | - ## | ||
| 23 | - ## Counted in chrome rows rather than points in the Clojure, because a | ||
| 24 | - ## terminal's row is one cell and a window's is 34. There is no terminal any | ||
| 25 | - ## more, so this is the window's number. | ||
| 26 | - 46 | ||
| 27 | - | ||
| 28 | func tabScreen*(s: State, title0, scrollKey: string, | 19 | func tabScreen*(s: State, title0, scrollKey: string, |
| 29 | body: varargs[Node]): Node = | 20 | body: varargs[Node]): Node = |
| 30 | ## One of the three screens the tab bar moves between: the title at the top, | 21 | ## One of the three screens the tab bar moves between: the title at the top, |
| @@ -34,14 +25,14 @@ func tabScreen*(s: State, title0, scrollKey: string, | |||
| 34 | ## middle. As pages they were centred columns of their own widths with the | 25 | ## middle. As pages they were centred columns of their own widths with the |
| 35 | ## tabs wherever the content happened to end, and every switch resized the | 26 | ## tabs wherever the content happened to end, and every switch resized the |
| 36 | ## screen under the pointer. | 27 | ## screen under the pointer. |
| 37 | - var list = vbox(%*{"key": "list", "fillHeight": true}) | 28 | + var list = vbox(%*{"key": "list", "expand": true}) |
| 38 | var sc = scroll(%*{"scrollKey": scrollKey, "orientation": "vertical", | 29 | var sc = scroll(%*{"scrollKey": scrollKey, "orientation": "vertical", |
| 39 | - "reserve": belowList(s), "spacing": 8}) | 30 | + "spacing": 8}) |
| 40 | for b in body: | 31 | for b in body: |
| 41 | if not b.isNil: sc.children.add b | 32 | if not b.isNil: sc.children.add b |
| 42 | list.children.add sc | 33 | list.children.add sc |
| 43 | 34 | ||
| 44 | - vbox(%*{"spacing": 8, "margin": 12, "fillHeight": true}, | 35 | + vbox(%*{"spacing": 8, "margin": 12, "expand": true}, |
| 45 | title(title0), | 36 | title(title0), |
| 46 | list, | 37 | list, |
| 47 | vbox(%*{"key": "foot", "spacing": 8}, | 38 | vbox(%*{"key": "foot", "spacing": 8}, |
modified
nim/src/frq/textruns.nim +25 -0 | @@ -11,6 +11,7 @@ | ||
| 11 | 11 | ## drags long URLs off the left edge. |
| 12 | 12 | |
| 13 | 13 | import std/strutils |
| 14 | +from std/unicode import runeLen, runeSubStr | |
| 14 | 15 | |
| 15 | 16 | type |
| 16 | 17 | RunKind* = enum rkText, rkLink |
| @@ -92,3 +93,27 @@ func firstImageUrl*(text: string): string = | ||
| 92 | 93 | if low.endsWith(".png") or low.contains("/media/"): |
| 93 | 94 | return r.value |
| 94 | 95 | "" |
| 96 | + | |
| 97 | + | |
| 98 | +func summarise*(text: string, n: int): string = | |
| 99 | + ## One line of `text`, cut to `n` characters with an ellipsis. | |
| 100 | + ## | |
| 101 | + ## Two callers with the same need: a reply chip quoting what it answers, and | |
| 102 | + ## a room's last line in the conversation list. A pasted shell script or a | |
| 103 | + ## long link is a card's worth of text otherwise, and the cards stop reading | |
| 104 | + ## as a list of rooms. | |
| 105 | + ## | |
| 106 | + ## Runes and not bytes. A byte slice lands inside a multi-byte character and | |
| 107 | + ## makes mojibake where an ellipsis was wanted, which both copies of this got | |
| 108 | + ## wrong before a test with a hundred emoji in it found them. | |
| 109 | + var line = newStringOfCap(text.len) | |
| 110 | + var inSpace = false | |
| 111 | + for c in text: | |
| 112 | + if c in {' ', '\t', '\n', '\r'}: | |
| 113 | + if not inSpace: line.add ' ' | |
| 114 | + inSpace = true | |
| 115 | + else: | |
| 116 | + line.add c | |
| 117 | + inSpace = false | |
| 118 | + line = line.strip() | |
| 119 | + if line.runeLen > n: line.runeSubStr(0, n - 1) & "…" else: line | |
| @@ -11,6 +11,7 @@ | |||
| 11 | ## drags long URLs off the left edge. | 11 | ## drags long URLs off the left edge. |
| 12 | 12 | ||
| 13 | import std/strutils | 13 | import std/strutils |
| 14 | +from std/unicode import runeLen, runeSubStr | ||
| 14 | 15 | ||
| 15 | type | 16 | type |
| 16 | RunKind* = enum rkText, rkLink | 17 | RunKind* = enum rkText, rkLink |
| @@ -92,3 +93,27 @@ func firstImageUrl*(text: string): string = | |||
| 92 | if low.endsWith(".png") or low.contains("/media/"): | 93 | if low.endsWith(".png") or low.contains("/media/"): |
| 93 | return r.value | 94 | return r.value |
| 94 | "" | 95 | "" |
| 96 | + | ||
| 97 | + | ||
| 98 | +func summarise*(text: string, n: int): string = | ||
| 99 | + ## One line of `text`, cut to `n` characters with an ellipsis. | ||
| 100 | + ## | ||
| 101 | + ## Two callers with the same need: a reply chip quoting what it answers, and | ||
| 102 | + ## a room's last line in the conversation list. A pasted shell script or a | ||
| 103 | + ## long link is a card's worth of text otherwise, and the cards stop reading | ||
| 104 | + ## as a list of rooms. | ||
| 105 | + ## | ||
| 106 | + ## Runes and not bytes. A byte slice lands inside a multi-byte character and | ||
| 107 | + ## makes mojibake where an ellipsis was wanted, which both copies of this got | ||
| 108 | + ## wrong before a test with a hundred emoji in it found them. | ||
| 109 | + var line = newStringOfCap(text.len) | ||
| 110 | + var inSpace = false | ||
| 111 | + for c in text: | ||
| 112 | + if c in {' ', '\t', '\n', '\r'}: | ||
| 113 | + if not inSpace: line.add ' ' | ||
| 114 | + inSpace = true | ||
| 115 | + else: | ||
| 116 | + line.add c | ||
| 117 | + inSpace = false | ||
| 118 | + line = line.strip() | ||
| 119 | + if line.runeLen > n: line.runeSubStr(0, n - 1) & "…" else: line | ||
modified
nim/src/frq/ui.nim +10 -0 | @@ -102,6 +102,16 @@ func separator*(): Node = n("separator") | ||
| 102 | 102 | |
| 103 | 103 | func spacer*(size: int): Node = n("spacer", %*{"size": size}) |
| 104 | 104 | |
| 105 | +func paragraph*(children: varargs[Node]): Node = | |
| 106 | + ## Prose with links in it, wrapping as text rather than as boxes. | |
| 107 | + ## | |
| 108 | + ## Its own tag rather than an `hbox` with an `inline` flag, which is what it | |
| 109 | + ## was: a row and a paragraph share no layout at all — not the gaps, not the | |
| 110 | + ## alignment, not even how a child is built — so saying "row" and then | |
| 111 | + ## contradicting it with a prop meant the renderer had to check the | |
| 112 | + ## contradiction before every row it drew. | |
| 113 | + n("paragraph", newJObject(), @children) | |
| 114 | + | |
| 105 | 115 | func text*(body: string): Node = |
| 106 | 116 | ## Prose, as opposed to a `label`: wraps, and is the thing a message is. |
| 107 | 117 | n("text", %*{"text": body}) |
| @@ -102,6 +102,16 @@ func separator*(): Node = n("separator") | |||
| 102 | 102 | ||
| 103 | func spacer*(size: int): Node = n("spacer", %*{"size": size}) | 103 | func spacer*(size: int): Node = n("spacer", %*{"size": size}) |
| 104 | 104 | ||
| 105 | +func paragraph*(children: varargs[Node]): Node = | ||
| 106 | + ## Prose with links in it, wrapping as text rather than as boxes. | ||
| 107 | + ## | ||
| 108 | + ## Its own tag rather than an `hbox` with an `inline` flag, which is what it | ||
| 109 | + ## was: a row and a paragraph share no layout at all — not the gaps, not the | ||
| 110 | + ## alignment, not even how a child is built — so saying "row" and then | ||
| 111 | + ## contradicting it with a prop meant the renderer had to check the | ||
| 112 | + ## contradiction before every row it drew. | ||
| 113 | + n("paragraph", newJObject(), @children) | ||
| 114 | + | ||
| 105 | func text*(body: string): Node = | 115 | func text*(body: string): Node = |
| 106 | ## Prose, as opposed to a `label`: wraps, and is the thing a message is. | 116 | ## Prose, as opposed to a `label`: wraps, and is the thing a message is. |
| 107 | n("text", %*{"text": body}) | 117 | n("text", %*{"text": body}) |
modified
nim/src/frq_core.nim +1 -2 | @@ -26,7 +26,7 @@ | ||
| 26 | 26 | ## between this and the experiment that was deleted for being a facsimile. |
| 27 | 27 | |
| 28 | 28 | import std/[json, strutils, tables] |
| 29 | -import frq/[ircparse, trace, ui, cells, reducer, model, rooms, reactions] | |
| 29 | +import frq/[ircparse, trace, ui, cells, reducer, model, rooms] | |
| 30 | 30 | import frq/conn as tr |
| 31 | 31 | import frq/screens/connect as scConnectScreen |
| 32 | 32 | import frq/screens/chats as scChatsScreen |
| @@ -151,7 +151,6 @@ proc frq_conn_event*(): cstring {.exportc, dynlib.} = | ||
| 151 | 151 | # crossing are a tree going out and an event id coming back. |
| 152 | 152 | |
| 153 | 153 | proc currentTree(): string = |
| 154 | - maybeAutoconnect() | |
| 155 | 154 | ## Whichever screen the state says. `drain` first, so the tree Dart gets is |
| 156 | 155 | ## built after every line that had arrived when it asked — that is the whole |
| 157 | 156 | ## of the polling model, and why there is no callback into Dart. |
| @@ -26,7 +26,7 @@ | |||
| 26 | ## between this and the experiment that was deleted for being a facsimile. | 26 | ## between this and the experiment that was deleted for being a facsimile. |
| 27 | 27 | ||
| 28 | import std/[json, strutils, tables] | 28 | import std/[json, strutils, tables] |
| 29 | -import frq/[ircparse, trace, ui, cells, reducer, model, rooms, reactions] | 29 | +import frq/[ircparse, trace, ui, cells, reducer, model, rooms] |
| 30 | import frq/conn as tr | 30 | import frq/conn as tr |
| 31 | import frq/screens/connect as scConnectScreen | 31 | import frq/screens/connect as scConnectScreen |
| 32 | import frq/screens/chats as scChatsScreen | 32 | import frq/screens/chats as scChatsScreen |
| @@ -151,7 +151,6 @@ proc frq_conn_event*(): cstring {.exportc, dynlib.} = | |||
| 151 | # crossing are a tree going out and an event id coming back. | 151 | # crossing are a tree going out and an event id coming back. |
| 152 | 152 | ||
| 153 | proc currentTree(): string = | 153 | proc currentTree(): string = |
| 154 | - maybeAutoconnect() | ||
| 155 | ## Whichever screen the state says. `drain` first, so the tree Dart gets is | 154 | ## Whichever screen the state says. `drain` first, so the tree Dart gets is |
| 156 | ## built after every line that had arrived when it asked — that is the whole | 155 | ## built after every line that had arrived when it asked — that is the whole |
| 157 | ## of the polling model, and why there is no callback into Dart. | 156 | ## of the polling model, and why there is no callback into Dart. |
modified
nim/tests/tchat.nim +5 -5 | @@ -1,7 +1,7 @@ | ||
| 1 | 1 | ## The conversation screen. |
| 2 | 2 | |
| 3 | 3 | import std/[json, sequtils, strutils, tables, unicode, unittest] |
| 4 | -import frq/[ui, cells, model, reactions] | |
| 4 | +import frq/[ui, cells, model, textruns] | |
| 5 | 5 | import frq/screens/chat as cs |
| 6 | 6 | |
| 7 | 7 | proc find(node: Node, tag: string): seq[Node] = |
| @@ -29,11 +29,11 @@ proc withRoom(): State = | ||
| 29 | 29 | |
| 30 | 30 | suite "summarise": |
| 31 | 31 | test "collapses whitespace and cuts to fit": |
| 32 | - check cs.summarise("a\n b", 36) == "a b" | |
| 33 | - check cs.summarise("x".repeat(50), 10).runeLen == 10 | |
| 34 | - check cs.summarise("x".repeat(50), 10).endsWith("…") | |
| 32 | + check summarise("a\n b", 36) == "a b" | |
| 33 | + check summarise("x".repeat(50), 10).runeLen == 10 | |
| 34 | + check summarise("x".repeat(50), 10).endsWith("…") | |
| 35 | 35 | test "leaves a short line alone": |
| 36 | - check cs.summarise("short", 36) == "short" | |
| 36 | + check summarise("short", 36) == "short" | |
| 37 | 37 | |
| 38 | 38 | suite "the chat screen": |
| 39 | 39 | setup: |
| @@ -1,7 +1,7 @@ | |||
| 1 | ## The conversation screen. | 1 | ## The conversation screen. |
| 2 | 2 | ||
| 3 | import std/[json, sequtils, strutils, tables, unicode, unittest] | 3 | import std/[json, sequtils, strutils, tables, unicode, unittest] |
| 4 | -import frq/[ui, cells, model, reactions] | 4 | +import frq/[ui, cells, model, textruns] |
| 5 | import frq/screens/chat as cs | 5 | import frq/screens/chat as cs |
| 6 | 6 | ||
| 7 | proc find(node: Node, tag: string): seq[Node] = | 7 | proc find(node: Node, tag: string): seq[Node] = |
| @@ -29,11 +29,11 @@ proc withRoom(): State = | |||
| 29 | 29 | ||
| 30 | suite "summarise": | 30 | suite "summarise": |
| 31 | test "collapses whitespace and cuts to fit": | 31 | test "collapses whitespace and cuts to fit": |
| 32 | - check cs.summarise("a\n b", 36) == "a b" | 32 | + check summarise("a\n b", 36) == "a b" |
| 33 | - check cs.summarise("x".repeat(50), 10).runeLen == 10 | 33 | + check summarise("x".repeat(50), 10).runeLen == 10 |
| 34 | - check cs.summarise("x".repeat(50), 10).endsWith("…") | 34 | + check summarise("x".repeat(50), 10).endsWith("…") |
| 35 | test "leaves a short line alone": | 35 | test "leaves a short line alone": |
| 36 | - check cs.summarise("short", 36) == "short" | 36 | + check summarise("short", 36) == "short" |
| 37 | 37 | ||
| 38 | suite "the chat screen": | 38 | suite "the chat screen": |
| 39 | setup: | 39 | setup: |
modified
nim/tests/thandshake.nim +1 -1 | @@ -1,7 +1,7 @@ | ||
| 1 | 1 | ## CAP negotiation and the SASL payload. No network: every case here is a |
| 2 | 2 | ## line in and lines out. |
| 3 | 3 | |
| 4 | -import std/[base64, json, sets, strutils, unittest] | |
| 4 | +import std/[json, sets, strutils, unittest] | |
| 5 | 5 | import frq/[ircparse, atproto, handshake] |
| 6 | 6 | |
| 7 | 7 | proc caps0(): HashSet[string] = initHashSet[string]() |
| @@ -1,7 +1,7 @@ | |||
| 1 | ## CAP negotiation and the SASL payload. No network: every case here is a | 1 | ## CAP negotiation and the SASL payload. No network: every case here is a |
| 2 | ## line in and lines out. | 2 | ## line in and lines out. |
| 3 | 3 | ||
| 4 | -import std/[base64, json, sets, strutils, unittest] | 4 | +import std/[json, sets, strutils, unittest] |
| 5 | import frq/[ircparse, atproto, handshake] | 5 | import frq/[ircparse, atproto, handshake] |
| 6 | 6 | ||
| 7 | proc caps0(): HashSet[string] = initHashSet[string]() | 7 | proc caps0(): HashSet[string] = initHashSet[string]() |