From 2fd52b41e89f6253f89f3cc3ccf9d1861626c1a7 Mon Sep 17 00:00:00 2001 From: Chris Bobbe Date: Mon, 12 Jun 2023 17:38:13 -0700 Subject: [PATCH] model: Add wrapWithBacktickFence, to use with quote-and-reply For the corresponding logic used in the web app and zulip-mobile, see web/shared/src/fenced_code.ts. I believe the logic here differs from that in just this way: it follows the CommonMark spec more closely by disqualifying backtick-fence lines where the "info string" has a backtick, since that's not allowed: > If the info string comes after a backtick fence, it may not > contain any backtick characters. (The reason for this restriction > is that otherwise some inline code would be incorrectly > interpreted as the beginning of a fenced code block.) Regarding the new file lib/model/compose.dart, we do have existing code that could reasonably move here, but it's pretty simple. It's the code that gives the upload-file Markdown; see registerUploadStart and registerUploadEnd in [ContentTextEditingController]. Related: #116 --- lib/model/compose.dart | 103 +++++++++++++++++++ test/model/compose_test.dart | 190 +++++++++++++++++++++++++++++++++++ 2 files changed, 293 insertions(+) create mode 100644 lib/model/compose.dart create mode 100644 test/model/compose_test.dart diff --git a/lib/model/compose.dart b/lib/model/compose.dart new file mode 100644 index 00000000000..58fa0faf465 --- /dev/null +++ b/lib/model/compose.dart @@ -0,0 +1,103 @@ +import 'dart:math'; + +// +// Put functions for nontrivial message-content generation in this file. +// +// If it's complicated enough to need tests, it should go in here. +// + +// https://spec.commonmark.org/0.30/#fenced-code-blocks +final RegExp _openingBacktickFenceRegex = (() { + // Recognize a fence with "up to three spaces of indentation". + // Servers don't recognize fences that start with spaces, as of Server 7.0: + // https://chat.zulip.org/#narrow/stream/6-frontend/topic/quote-and-reply.20fence.20length/near/1588273 + // but that's a bug, since those fences are valid in the spec. + // Still, it's harmless to make our own fence longer even if the server + // wouldn't notice the internal fence that we're steering clear of, + // and if servers *do* start following the spec by noticing indented internal + // fences, then this client behavior will be nice. + const lineStart = r'^ {0,3}'; + + // The backticks, captured so we can see how many. + const backticks = r'(`{3,})'; + + // The "info string" plus (meaningless) leading or trailing spaces or tabs. + // It can't contain backticks. + const trailing = r'[^`]*$'; + return RegExp(lineStart + backticks + trailing, multiLine: true); +})(); + +/// The shortest opening backtick fence that's longer than any in [content]. +/// +/// Expressed as a number of backticks. +/// +/// Use this for quote-and-reply or anything else that requires wrapping +/// Markdown in a backtick fence. +/// +/// See the CommonMark spec, which Zulip servers should but don't always follow: +/// https://spec.commonmark.org/0.30/#fenced-code-blocks +int getUnusedOpeningBacktickFenceLength(String content) { + final matches = _openingBacktickFenceRegex.allMatches(content); + int result = 3; + for (final match in matches) { + result = max(result, match[1]!.length + 1); + } + return result; +} + +/// Wrap Markdown [content] with opening and closing backtick fences. +/// +/// For example, for this Markdown: +/// +/// ```javascript +/// console.log('Hello world!'); +/// ``` +/// +/// this function, with `infoString: 'quote'`, gives +/// +/// ````quote +/// ```javascript +/// console.log('Hello world!'); +/// ``` +/// ```` +/// +/// See the CommonMark spec, which Zulip servers should but don't always follow: +/// https://spec.commonmark.org/0.30/#fenced-code-blocks +// In [content], indented code blocks +// ( https://spec.commonmark.org/0.30/#indented-code-blocks ) +// and code blocks fenced with tildes should make no difference to the +// backtick fences we choose here; this function ignores them. +String wrapWithBacktickFence({required String content, String? infoString}) { + assert(infoString == null || !infoString.contains('`')); + assert(infoString == null || infoString.trim() == infoString); + + StringBuffer resultBuffer = StringBuffer(); + + // (A) Why not panic on dangling opening fences + // (ones without a corresponding closing fence)? + // Because the spec allows leaving the closing fence implicit: + // > If the end of the containing block (or document) is reached + // > and no closing code fence has been found, + // > the code block contains all of the lines after the opening code fence + // > until the end of the containing block (or document). + // + // (B) Why not look for dangling closing fences (ones without an opening fence)? + // Because technically there's no such thing: + // they would be indistinguishable from dangling opening fences, + // and parsers (and this function) will treat them that way. + final fenceLength = getUnusedOpeningBacktickFenceLength(content); + + resultBuffer.write('`' * fenceLength); + if (infoString != null) { + resultBuffer.write(infoString); + } + resultBuffer.write('\n'); + resultBuffer.write(content); + resultBuffer.write('\n'); + resultBuffer.write('`' * fenceLength); + resultBuffer.write('\n'); + return resultBuffer.toString(); +} + +// TODO more, like /near links to messages in conversations +// (also to be used in quote-and-reply) diff --git a/test/model/compose_test.dart b/test/model/compose_test.dart new file mode 100644 index 00000000000..68aadf9ced7 --- /dev/null +++ b/test/model/compose_test.dart @@ -0,0 +1,190 @@ +import 'package:checks/checks.dart'; +import 'package:test/scaffolding.dart'; +import 'package:zulip/model/compose.dart'; + +void main() { + group('wrapWithBacktickFence', () { + /// Check `wrapWithBacktickFence` on example input and expected output. + /// + /// The intended input (content passed to `wrapWithBacktickFence`) + /// is straightforward to infer from `expected`. + /// To do that, this helper takes `expected`, validates it + /// (as a courtesy for the test author), and removes the opening and + /// closing fences. + /// + /// Then we have the input to the test, as well as the expected output. + void checkFenceWrap(String expected, {String? infoString}) { + final lines = expected.split('\n'); + final firstLineMatch = RegExp(r'^(`{3,})[^`]*$').allMatches(lines.removeAt(0)).singleOrNull; + assert(firstLineMatch != null, + 'test error: opening fence not found in `expected`'); + assert(lines.removeAt(lines.length - 1) == '', + 'test error: `expected` should end with a newline'); + final openingFenceLength = firstLineMatch![1]!.length; + assert(RegExp(r'^`{' + openingFenceLength.toString() + r'}$').hasMatch(lines.removeAt(lines.length - 1)), + 'test error: closing fence not found in `expected`'); + final content = lines.join('\n'); + check(wrapWithBacktickFence(content: content, infoString: infoString)).equals(expected); + } + + test('single line with no code blocks', () { + checkFenceWrap(''' +``` +hello world +``` +'''); + }); + + test('multiple lines with no code blocks', () { + checkFenceWrap(''' +``` +hello +world +``` +'''); + }); + + test('three-backtick block', () { + checkFenceWrap(''' +```` +hello +``` +code +``` +world +```` +'''); + }); + + test('multiple three-backtick blocks; one has info string', () { + checkFenceWrap(''' +```` +hello +``` +code +``` +world +```javascript +// more code +``` +```` +'''); + }); + + test('whitespace around info string', () { + checkFenceWrap(''' +```` +``` javascript +// hello world +``` +```` +'''); + }); + + test('four-backtick block', () { + checkFenceWrap(''' +````` +```` +hello world +```` +````` +'''); + }); + + test('five-backtick block', () { + checkFenceWrap(''' +`````` +````` +hello world +````` +`````` +'''); + }); + + test('three-, four-, and five-backtick blocks', () { + checkFenceWrap(''' +`````` +``` +hello world +``` + +```` +hello world +```` + +````` +hello world +````` +`````` +'''); + }); + + test('dangling opening fence', () { + checkFenceWrap(''' +````` +````javascript +// hello world +````` +'''); + }); + + test('code blocks marked by indentation or tilde fences don\'t affect result', () { + checkFenceWrap(''' +``` + // hello world + +~~~~~~ +code +~~~~~~ +``` +'''); + }); + + test('backtick fences may be indented up to three spaces', () { + checkFenceWrap(''' +```` + ``` +```` +'''); + checkFenceWrap(''' +```` + ``` +```` +'''); + checkFenceWrap(''' +```` + ``` +```` +'''); + // but at 4 spaces of indentation it no longer counts: + checkFenceWrap(''' +``` + ``` +``` +'''); + }); + + test('fence ignored if info string has backtick', () { + checkFenceWrap(''' +``` +```java`script +hello +``` +'''); + }); + + test('with info string', () { + checkFenceWrap(infoString: 'info', ''' +`````info +``` +hello +``` +info +````python +hello +```` +````` +'''); + }); + }); +}