Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 16 additions & 4 deletions packages/gazzodown/src/blocks/OrderedListBlock.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,23 @@ type OrderedListBlockProps = {
items: MessageParser.ListItem[];
};

const renderChildren = (children: MessageParser.ListItem[]): ReactElement => (
<ol style={{ paddingInlineStart: '1.5rem' }}>
{children.map((item, index) => (
<li key={index} value={item.number}>
<InlineElements>{item.value}</InlineElements>
{item.children?.length ? renderChildren(item.children) : null}
</li>
))}
</ol>
);

const OrderedListBlock = ({ items }: OrderedListBlockProps): ReactElement => (
<ol>
{items.map(({ value, number }, index) => (
<li key={index} value={number}>
<InlineElements>{value}</InlineElements>
<ol style={{ paddingInlineStart: '1.5rem' }}>
{items.map((item, index) => (
<li key={index} value={item.number}>
<InlineElements>{item.value}</InlineElements>
{item.children?.length ? renderChildren(item.children) : null}
</li>
))}
</ol>
Expand Down
14 changes: 13 additions & 1 deletion packages/gazzodown/src/blocks/UnorderedListBlock.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,23 @@ type UnorderedListBlockProps = {
items: MessageParser.ListItem[];
};

const renderChildren = (children: MessageParser.ListItem[]): ReactElement => (
<ul style={{ paddingInlineStart: '1.5rem' }}>
{children.map((item, index) => (
<li key={index}>
<InlineElements>{item.value}</InlineElements>
{item.children?.length ? renderChildren(item.children) : null}
</li>
))}
</ul>
);

const UnorderedListBlock = ({ items }: UnorderedListBlockProps): ReactElement => (
<ul>
<ul style={{ paddingInlineStart: '1.5rem' }}>
{items.map((item, index) => (
<li key={index}>
<InlineElements>{item.value}</InlineElements>
{item.children?.length ? renderChildren(item.children) : null}
</li>
Comment thread
coderabbitai[bot] marked this conversation as resolved.
))}
</ul>
Expand Down
2 changes: 2 additions & 0 deletions packages/message-parser/src/definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ export type ListItem = {
type: 'LIST_ITEM';
value: Inlines[];
number?: number;
indent?: number;
children?: ListItem[];
};

export type Tasks = {
Expand Down
66 changes: 50 additions & 16 deletions packages/message-parser/src/grammar.pegjs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,26 @@ let skipBold = false;
let skipItalic = false;
let skipStrikethrough = false;
let skipReferences = false;

function buildNestedList(flatItems) {
const root = [];
const stack = [{ level: -1, children: root }];

for (const item of flatItems) {
const level = item.indent ?? 0;

// Pop stack until we find a parent whose level < current
while (stack.length > 1 && stack[stack.length - 1].level >= level) {
stack.pop();
}

const node = { ...item, children: [] };
stack[stack.length - 1].children.push(node);
stack.push({ level, children: node.children });
}

return root;
}
Comment on lines +46 to +64
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

CommonMark specification how tabs are interpreted for list indentation

💡 Result:

In CommonMark, tabs are treated specially when they affect block structure (including list indentation): 1) General rule for tabs Tabs are not automatically expanded to spaces. However, in contexts where whitespace defines block structure, a tab behaves as if it were replaced by spaces using a tab stop of 4 characters. 2) How this impacts list indentation When deciding whether a line is part of a list item (including sublist nesting / continuation blocks), CommonMark’s list-item parsing uses indentation based on the “width” of the list marker plus the following required spaces (and the general tab rule applies in the whitespace that determines indentation). A tab used to indent a continuation paragraph within a list item is equivalent to indenting with four spaces (because that tab stop is 4 characters). The spec includes an explicit example showing that effect. Practical implication: to nest/align content under list items, you can rely on tabs contributing indentation in 4-character steps; e.g., a single tab provides the same indentation “width” as four spaces for these list-structure decisions. Sources: CommonMark spec section on tabs (2.2) and the list-item indentation example demonstrating a tab as equivalent to four spaces.

Citations:


🏁 Script executed:

# First, let's locate and examine the grammar.pegjs file
cd packages/message-parser && ls -la src/

# Read the full grammar.pegjs file to see the context around lines 46-64
wc -l src/grammar.pegjs

Repository: RocketChat/Rocket.Chat

Length of output: 634


🏁 Script executed:

# Read the grammar.pegjs file, focusing on the buildNestedList function and surrounding context
head -100 packages/message-parser/src/grammar.pegjs | tail -60

Repository: RocketChat/Rocket.Chat

Length of output: 1167


🏁 Script executed:

# Search for where indent is captured and used in list-item rules
rg -n "indent" packages/message-parser/src/grammar.pegjs -A 2 -B 2

Repository: RocketChat/Rocket.Chat

Length of output: 940


🏁 Script executed:

# Look for OrderedListItem, UnorderedListHyphenItem, UnorderedListAsteriskItem rules
rg -n "OrderedListItem|UnorderedListHyphenItem|UnorderedListAsteriskItem" packages/message-parser/src/grammar.pegjs -A 5 -B 1

Repository: RocketChat/Rocket.Chat

Length of output: 1268


Indentation must use tab-stop column width, not character count, to match CommonMark behavior.

buildNestedList receives item.indent from the three list-item rules (lines 207, 217, 221), all of which use indent.length where indent is captured as $([ \t]*). This treats a tab as 1 character, but CommonMark specifies that tabs expand to the next 4-column tab stop. Mixed input like \t- a (indent.length = 1) and - b (indent.length = 4) will produce inconsistent nesting hierarchies, even though both represent 4-column indentation in CommonMark.

Use a tab-aware function to compute column width:

Example implementation
const indentWidth = (s) => {
  let c = 0;
  for (const ch of s) c += ch === '\t' ? 4 - (c % 4) : 1;
  return c;
};

Then replace indent.length with indentWidth(indent) in all three list-item rules.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/message-parser/src/grammar.pegjs` around lines 46 - 64, The list
nesting currently uses raw indent.length which treats tabs as one character; add
a tab-aware function named indentWidth(s) that computes columns with tab stops
of 4 (tabs expand to next 4-column boundary) and update the three list-item
rules that currently set item.indent using indent.length to instead set
item.indent = indentWidth(indent); leave buildNestedList as-is (it reads numeric
item.indent) so nesting uses the corrected column widths; ensure the new
indentWidth is referenced by name and available to the grammar so those rules
can call it.

}}

Start
Expand Down Expand Up @@ -174,30 +194,44 @@ TaskFlag = "x" { return true; } / " " { return false; }
* 3. Item Three
*
*/
OrderedList = items:OrderedListItem+ { return orderedList(items); }

OrderedListItem = number:Digits "." [ \t]+ text:Inline { return listItem(text, parseInt(number, 10)); }

/**
*
* Unordered List
* Ordered List (with nesting support)
* e.g:
* - Item One
* - Item Two
* * Item Three
* * Item Four
*
* 1. Item One
* 1. Nested Item
*/
UnorderedList = items:(UnorderedListHyphenItem+ / UnorderedListAsteriskItem+) { return unorderedList(items); }

UnorderedListHyphenItem = "-" [ \t]+ text:Inline { return listItem(text); }
OrderedList = items:OrderedListItem+ { return orderedList(buildNestedList(items)); }

UnorderedListAsteriskItem = "*" [ \t]+ text:UnorderedListItemContent { return listItem(text); }
OrderedListItem
= indent:$([ \t]*) number:Digits "." [ \t]+ text:Inline
{ return listItem(text, parseInt(number, 10), indent.length); }

UnorderedListItemContent = value:UnorderedListItemContentItem+ !"*" EndOfLine? { return reducePlainTexts(value); }
/**
* Unordered List (with nesting support)
*/
UnorderedList = items:(UnorderedListHyphenItem / UnorderedListAsteriskItem)+
{ return unorderedList(buildNestedList(items)); }

UnorderedListHyphenItem
= indent:$([ \t]*) "-" [ \t]+ text:Inline
{ return listItem(text, undefined, indent.length); }

UnorderedListAsteriskItem
= indent:$([ \t]*) "*" [ \t]+ text:UnorderedListAsteriskItemContent
&{
const plainText = text.map((item) => item.value ?? '').join('');
return plainText !== '*' && !(plainText.endsWith('*') && !plainText.startsWith('*'));
}
{ return listItem(text, undefined, indent.length); }

UnorderedListItemContentItem = InlineItemPattern / !"*" @Any
UnorderedListAsteriskItemContent
= value:UnorderedListAsteriskContentItem+ EndOfLine?
{ return reducePlainTexts(value); }

UnorderedListAsteriskContentItem
= !EndOfLine !("*" [ \t]) @InlineItemPattern
/ !EndOfLine !("*" [ \t]) @Any
Comment thread
coderabbitai[bot] marked this conversation as resolved.
/**
*
* KaTex
Expand Down
6 changes: 4 additions & 2 deletions packages/message-parser/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,10 +131,12 @@ export const orderedList = generate('ORDERED_LIST');

export const unorderedList = generate('UNORDERED_LIST');

export const listItem = (text: Inlines[], number?: number): ListItem => ({
export const listItem = (text: Inlines[], number?: number, indent = 0): ListItem => ({
type: 'LIST_ITEM',
value: text,
...(number !== undefined && { number }),
indent,
children: [],
});

export const mentionUser = (() => {
Expand Down Expand Up @@ -191,7 +193,7 @@ export const reducePlainTexts = (values: Paragraph['value']): Paragraph['value']
let needsSlowPath = false;
for (let i = 0; i < flattenableValues.length; i++) {
const v = flattenableValues[i];
if (Array.isArray(v) || (v as Inlines).type === 'EMOJI') {
if (Array.isArray(v) || v.type === 'EMOJI') {
needsSlowPath = true;
break;
}
Expand Down
2 changes: 2 additions & 0 deletions packages/message-parser/tests/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ export const unorderedList = generate('UNORDERED_LIST');
export const listItem = (value: unknown[], number?: number) => ({
type: 'LIST_ITEM' as const,
value,
children: [],
indent: 0,
...(number !== undefined ? { number } : {}),
});

Expand Down
9 changes: 7 additions & 2 deletions packages/message-parser/tests/unorderedList.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,15 @@ test.each([
* *Fourth item*
`.trim(),
[
unorderedList([listItem([plain('First item')])]),
unorderedList([listItem([plain('Second item')]), listItem([plain('Third item')]), listItem([bold([plain('Fourth item')])])]),
unorderedList([
listItem([plain('First item')]),
listItem([plain('Second item')]),
listItem([plain('Third item')]),
listItem([bold([plain('Fourth item')])]),
]),
],
],
[`* A * B`, [unorderedList([listItem([plain('A ')]), listItem([plain('B')])])]],
// [
// `
// * First item
Expand Down