Add the match and parse functions for spoiler text
AbandonedPublic
Actions

Authored by rohan on Oct 11 2022, 11:28 AM.

Details

Reviewers

atul
ginsu
• abosh
tomek
ashoat
kamil

Summary

Added the match and parse functions in order to execute the RegEx in the previous diff (in the stack), so we can then capture the spoiler text and transform it later.

Linear Issue: https://linear.app/comm/issue/ENG-1982/set-up-the-match-and-parse-functions-in-markdownjs-in-the-lib

Depends on D5335

Test Plan

The testing for this will come in the subsequent diff, where we
will add the spoiler rule to the markdown rules in native.

Diff Detail

Repository

rCOMM Comm

Branch

spoiler_2_match_and_parse

Lint

No Lint Coverage

Unit

No Test Coverage

Event Timeline

rohan created this revision.Oct 11 2022, 11:28 AM

Herald added subscribers: tomek, ashoat. · View Herald TranscriptOct 11 2022, 11:28 AM

rohan edited the summary of this revision. (Show Details)Oct 11 2022, 11:34 AM

rohan added a parent revision: D5335: [lib] Set up RegEx to match text between two spoiler characters.

Harbormaster completed remote builds in B12735: Diff 17496.Oct 11 2022, 11:38 AM

rohan requested review of this revision.Oct 11 2022, 11:38 AM

rohan edited the summary of this revision. (Show Details)Oct 11 2022, 11:42 AM

rohan added a child revision: D5347: Add spoiler rule to native in fullMarkdownRules.

looks good to me! Everything makes sense and seems to follow what was in the simple-markdown docs

atul added reviewers: tomek, ashoat.Oct 11 2022, 5:46 PM

Adding @kamil, who has looked at match/parse functions in SimpleMarkdown recently. Some questions inline

lib/shared/markdown.js
255	We seem to use `SimpleMarkdown.inlineRegex(spoilerRegex)` for this elsewhere in the codebase
264–273	Noticed a function `SimpleMarkdown.parseInline` that seems to match `inlineRegex` and we use somewhere else in the codebase. I'm wondering whether or not we should just use that. Can you explain what properties differentiate that function from the approach you've taken here, eg. does setting `state.inline` affect how nested Markdown might be processed?

This revision now requires changes to proceed.Oct 11 2022, 7:32 PM

Discussed in my sync with @ashoat - there's no need for custom match and parse methods, we can instead use the ones from SimpleMarkdown, and I will update the subsequent diff with the new spoiler rule

rohan mentioned this in D8834: [lib] Introduce chat mention match and parse utilities.Aug 21 2023, 5:32 PM

Revision Contents
Changeset List

Path

Size

lib/

shared/

markdown.js

22 lines

Diff 17496

View Options

lib/shared/markdown.js

Show First 20 Lines • Show All 55 Lines • ▼ Show 20 Lines	networks:
- services-net		- services-net
build:		build:
dockerfile: services/identity/Dockerfile		dockerfile: services/identity/Dockerfile
context: ../		context: ../
image: commapp/identity-server:0.1		image: commapp/identity-server:0.1
container_name: identity-server		container_name: identity-server
ports:		ports:
- "${COMM_SERVICES_PORT_IDENTITY}:50051"		- "${COMM_SERVICES_PORT_IDENTITY}:50051"
		# localstack
		localstack:
		image: localstack/localstack
		container_name: localstack
		hostname: localstack
		ports:
		- "4566:4566"
		environment:
		- SERVICES=s3,dynamodb
		- DATA_DIR=${TMPDIR}/localstack/data
		- HOSTNAME_EXTERNAL=localstack
		volumes:
		jimUnsubmitted Not Done Inline Actions Why bind mount a temp directory instead of using a data volume? It's just gonna create issues with permissions. Also ${TMPDIR} is not used to refer to a path in both the host and the container which is a bad idea. jim: Why bind mount a temp directory instead of using a data volume? It's just gonna create issues…
		- ${TMPDIR}:/tmp/localstack
		networks:
		- services-net

Add the match and parse functions for spoiler textAbandonedPublicActions