Шубин Игорь #263

Nevisinn · 2025-11-05T09:25:54Z

Идея следующая: посимвольно парсим, на каждый символ находим свой парсер в ParserSelector (думаю с помощью switch возвращать соответствующий символу парсер ), затем проверять, можно ли спарсить тег, если да, то парсим и получаем ноду, которую добавляем в древовидную структуру MarkdownDocument, если не парсится, то это текст, его так же добавляем в дерево. После того, как проитерируемся по всем элементам, у дерева вызовем метод ToHtml, он обойдет все элементы в глубину, вызовет у нод ToHtml и сформирует строку в формате html.
MarkdownDocument и ноды в папке Nodes - это паттерн композит. Он необходим, чтобы отслеживать взаимодействие и вложенность тегов.

Yrwlcm

Кажется если начать реализовывать текущее решение, то на этапе парсинга текста станет сильно больно. Свои мысли по решению написал. Но не исключаю, что я пропустил ключевую идею, из-за которой твое решение норм, так что я открыт к обсуждению :)

Предлагаю тебе пересмотреть решение и перейти на другой способ парсинга синтаксиса языков (который, вроде бы, и используется в основном) и поискать про него информацию

Там используется токенайзер, который текст превращает в набор токенов, с которыми будет удобнее работать и единственный парсер, который как раз эти токены читает и на их основе собирает как раз дерево из нод, которое ты уже используешь.

Так у тебя будет централизованная логика в парсере без дублирования и её проще будет менеджментить. А еще результат будет линейный от входа

Yrwlcm · 2025-11-05T13:02:18Z

cs/Markdown/MarkdownDocument.cs

+public class MarkdownDocument
+{
+    private DocumentNode root;
+    private MarkdownNode currentNode;
+
+    public MarkdownDocument()
+    {
+        root = new DocumentNode(null, "");
+    }
+
+    public void AddNode(MarkdownNode node)
+    {
+
+    }
+
+    public string ToHtml()
+    {
+        throw new NotImplementedException();
+    }
+}


На самом деле не понял, зачем нам нужен этот класс и как он будет работать, есть только додумки. Мы хотим построить дерево, у которого потом вызовем .ToHtml() и пускай оно само нам строчку вернет - это правильно, так по идее и должно быть

Предполагаю у нас для этого есть DocumentNode и мы все ноды в него будем сворачивать. Нооо как раз нода документа - сейчас нигде не фигурирует в коде и кажется либо она, либо этот класс лишний

Еще не совсем понял как у нас в этом классе документа будет реализовано AddNode списков тут нет, а вызывать у чего-то дочернего по цепочке AddNode, кажется пока сомнительно, но может я не до конца понял что ты хочешь сделать

Планировалась логика добавления нод, для этого в поле есть DocumentNode, но в принципе я согласен, класс излишен

Yrwlcm · 2025-11-05T13:09:29Z

cs/Markdown/Nodes/Interfaces/InternalMarkdownNode.cs

+namespace Markdown.Nodes.Interfaces;
+
+public abstract class InternalMarkdownNode : MarkdownNode
+{
+    protected readonly List<MarkdownNode> children = [];
+    public override void Add(MarkdownNode node)
+    {
+
+    }
+
+    public override List<MarkdownNode> GetChildren()
+        => children;
+
+    protected InternalMarkdownNode(MarkdownNode? parent, string value) : base(parent, value)
+    {
+    }
+}


Этот класс сейчас тоже кажется избыточным. Вот мои доводы почему:

Мы хотим, чтобы наш класс MarkdownNode и все его реализации умели делать следующее:

Отдельно выделю, т.к. коммент не про это

Хранить свое значение

Знать своего родителя

Добавлять в себя элемент

Хранить весь список элементов

Будто бы сразу же приходит в голову список - ровно то, для чего он нужен. И тогда кажется логичным пускай сразу в MarkdownNode и реализуется этот список, для дерева это тоже подходит. А тут мы получается вынесли реализацию списка в отдельный класс и это прямо слишком абстракция

Сходу в голову не приходит адекватный сценарий, в котором мы решим не использовать список, а например словарь, потому что нам не нужны функции словаря, нам даже уметь доставать по индексу не нужно. А ради таких призрачных сценариев не стоит усложнять код, а просто следовать принципу KISS :)

Ответил ниже

Нуу, в целом теперь окей, наверное если мы оставим реализацию того, что у нас листья наследуются от MarkdownNode то тогда и этот класс можно оставить с реализацией списка

Yrwlcm · 2025-11-05T13:25:07Z

cs/Markdown/Nodes/Interfaces/LeafMarkdownNode.cs

+namespace Markdown.Nodes.Interfaces;
+
+public abstract class LeafMarkdownNode : MarkdownNode
+{
+    protected LeafMarkdownNode(MarkdownNode? parent, string value) : base(parent, value)
+    {
+    }
+
+    public override void Add(MarkdownNode node)
+    {
+        throw new NotImplementedException();
+    }
+}


Тут кажется чтобы был правильный смысл, нужно поменять наследование. Мы ведь сейчас можем в листы дерева добавлять еще ноды, т.к. мне класс MarkdownNode это позволяет и я могу при желании к нему с апкастить. Пусть тогда у нас все наследуется от листа, где только значение, родитель и tohtml, а потом не листовые ноды с их списками

Ты еще занес в крайние ноды ImageNode, но что если я захочу написать например вот так

[my __bold__ image](src="http something ...")

Мы не сможем добавлять в листы дерева (или если сделаем апкаст) еще ноды, т.к там будет эксепшн. В паттерне composite у листьев оставляют методы пустыми или выбрасывают эксепшн. Еще как вариант, сделать флаг, является ли нода листом.

Насчет поменять наследование, в теории должно сработать. И как ты говорил выше, можно отказаться от такого подхода вообще, оставив только список, и в данной задаче, как я понимаю, можно не усложнять?

Вот эта часть мне не очень нравится в этом решении, хоть это и по паттерну

там будет эксепшн

Точнее, я не понимал в каком контексте мы будем этот метод .Add вызывать и каким способом мы будем понимать можем мы это сделать или нет. Если бы нам пришлось заворачивать что-то в трай кетч, это было бы плохо. Т.к. конкретно это исключение - часть ожидаемой логики, что у нас будут листья дерева. А не что-то непредвиденное

С учетом того, что переделали решение на токенайзер и парсер и теперь парсер должен сам понимать у кого что он может вызывать, чтобы собрать дерево - теперь такой подход норм и его можно оставить)

Yrwlcm · 2025-11-05T13:47:50Z

cs/Markdown/Parsing/Interfaces/IParser.cs

+using Markdown.Nodes.Interfaces;
+
+namespace Markdown.Parsing.Interfaces;
+
+public interface IParser
+{
+    bool CanParse(string text);
+    MarkdownNode Parse(string text);
+}


Идея с парсерами кажется плохой и если ты её начнешь реализовывать будет сильно больно:

Это уже не линейная обработка входящего текста, у нас сложность рендера вырастет до
O(N * M), где N - длина выхода, M - кол-во парсеров.
Так как мы каждый символ должны будем прогнать через все наши парсеры, иначе что-то можем пропустить

Непонятно как выбрать парсер в случае _ курсива и __ жирного текста

Как мы будем обрабатывать пересечения тегов? А вложенность? (а если и будем, то опять кажется каждый парсер должен знать про особенности других тегов, чтобы их обработать или нет)

Сейчас в решении непонятно в какой момент из текста появляются конкретные ноды (типо Bold или Header и т.п.) IParser этого не подразумевает

Потерялась логика с экранированием символов, возможно она закладывалась в каждый парсер, но тогда это тоже сильное дублирование

Yrwlcm · 2025-11-08T06:11:17Z

cs/Markdown/MarkdownParser.cs

+    private BoldNode ParseStrong()
+    {
+        throw new NotImplementedException();
+    }
+
+    private ItalicNode ParseEmphasis()


Придираюсь) Но кажется чутка странным, что мы вызываем ParseStrong а на выходе BoldNode, лучше одно название зафиксировать. Для ItalicNode ParseEmphasis тоже

Yrwlcm · 2025-11-08T06:14:03Z

cs/Markdown/TokenType.cs

@@ -0,0 +1,16 @@
+namespace Markdown;
+
+public enum TokenType


Закину этот пример на подумать, кажется сейчас с ним могут быть трудности при парсинге

Однако выделять часть слова они могут: и в _нач_але, и в сер_еди_не, и в кон_це._ В то же время выделение в ра_зных сл_овах не работает.

Yrwlcm

Честно - я не стал смотреть реализацию парсеров и детально смотреть реализацию парсера. Это сложно читать и разбираться, как и что работает.

Одна из первых проблем этого решения - ParserContext, у нас относительно непредсказуемым образом он меняется то тут, то в парсерах. Каждый парсер еще и тип текущего токена может поменять в контексте, это прямо 100% нет, хотя бы потому, что мы точно можем этого избежать.

Если приходится менять тип текущего токена - значит токенайзер плохо выполнил свою работу. Добавь в него тогда нужную валидацию, а правда ли это токен или все же текст, посмотреть на соседние символы в токенайзере - норм. Считай что поток токенов к тебе приходит - иммутабельный, по нему можно только ходить, но не менять содержимое

Возможно ты еще не совсем понял, зачем нам вообще этот подход нужен, можешь еще поискать в интернете, но основная идея такая:

Мы можем внутри парсинга одних токенов, вызывать парсинг других. Ну т.е. вместо 2 огромных свичей, сначала в выборе парсера, потом в BuildMarkdownDocument и сложной логики запрятанной в каждом из парсеров. Будет один парсер с методом ParseNode().

В ней уже будет свич, или например методы TryReadHeaderNode, TryReadItalicNode и т.п. подряд, если какой-то выдал true и смог спарсить - окей, и идем дальше форычем по всем токенам, пока они не кончатся.
На примере метода заголовка, что он должен делать внутри

TryReadHeaderNode()
{
	ReadHashToken()
	ReadSpaceToken()
	ParseNode()
	ReadEndOfLine()
}

В момент вызова ParseNode мы рекурсивно зайдем в этот метод и считаем например все Italic и Bold внутри.

Если хочется понять, как обрабатывать то, что у нас в одном токене, не могут быть другие, как например в хедере не может быть еще хедер, можно в метод передавать ридонли лист с набором родителей, в которые он вложен, и проверять, что если у нас вызван TryReadHeader внутри уже хедера, то возвращать сразу False и т.д.

И еще не хватает отдельных тестов на токенайзер и парсер и желательно рендер, помимо тестов сразу на все. То, что все вместе работает - это хорошо, но мы никак не фиксируем логику конкретных частей, а значит мы не понимаем в какой момент и что мы сломали.

Yrwlcm · 2025-11-19T16:50:51Z

cs/Markdown/MarkdownTokenizer.cs

+    private int position;
+    private readonly string markdownText;
+    private char Prev => position - 1 < 0 ? ' ' : markdownText[position - 1];
+    private char Next => position + 1 == markdownText.Length ? ' ' : markdownText[position + 1];
+    public MarkdownTokenizer(string markdownText)
+    {
+        this.markdownText = markdownText;
+    }
+    public List<Token> Tokenize()


Тут все слиплось - приватные поля с конструктором и методом, добавь отступов

Yrwlcm · 2025-11-19T16:51:52Z

cs/Markdown/MarkdownTokenizer.cs

+
+        position++;
+        nextSymbol = Next;
+        if (prevSymbol != ' ' && nextSymbol != ' ' && nextSymbol != '_')


prevSymbol - вводящее в заблуждение название переменной. Не сразу понял, что это символ до нижних подчеркиваний в данном контексте, а не предыдущий символ. Хотел написать, зачем тут проверяем предыдущий символ, он ведь всегда _, если дошли до этого участка кода

Еще не нужны переменные которые дублируют свойства Next и Prev, зачем тогда эти свойства добавляли

Yrwlcm · 2025-11-19T16:52:14Z

cs/Markdown/MarkdownTokenizer.cs

+
+    }
+
+    private Token TokenizeImage()


Этот метод не нужен, т.к. все что он делает - это свич, а снаружи мы 5 раз его вызываем, чтобы получить конкретный результат свича

Yrwlcm · 2025-11-19T16:54:59Z

cs/Markdown/MarkdownTokenizer.cs

+    private bool IsSpecialSymbol(char symbol)
+        => specialsSymbols.Contains(symbol);
+
+    private readonly HashSet<char> specialsSymbols = ['#', '_', '!', '[', ']', '(', ')', ' ', '\n', '\\'];


А почему это положил в конец класса? И символы точно стоит вынести в общее место, вместе с теми что лежат внутри метода токенайзера. Туда добавил, сюда забыл, попробуй разберись почему не работает

Yrwlcm · 2025-11-19T17:05:24Z

cs/Markdown/ParseSelector.cs

+            case TokenType.LBracket:
+            case TokenType.LParenthesis:
+            case TokenType.RBracket:
+            case TokenType.RParenthesis:
+            case TokenType.Exclamation:


Кажется парсер не должен так работать, хотя бы с точки зрения того, что мы выбираем парсер на основе закрывающих символов и без какого-либо контекста

Т.е. наше решение развалится или превратится в еще большего монстра, если у нас появятся разные конструкции, которые используют одинаковый закрывающий символ, ну или разный контекст. В маркдауне конечно не факт, что такое встретится, но если посмотрим на решение - как подход к обработке формальных языков

Yrwlcm · 2025-11-19T17:29:43Z

cs/Markdown/Tests/MdTests.cs

+    [TestCase("![Cat](https://i.pinimg.com/originals/f5/ef/a6/f5efa6a__5b2__c76c038ef0c8d2502fd2f6.jpg)",
+        "<img src =\"https://i.pinimg.com/originals/f5/ef/a6/f5efa6a__5b2__c76c038ef0c8d2502fd2f6.jpg\" alt=\"Cat\">", TestName = "UrlWithBold")]
+    [TestCase("![[][][]](https://i.pinimg.com/originals/f5/ef/a6/f5efa6a5b2c76c038ef0c8d2502fd2f6.jpg)",
+        "<img src =\"https://i.pinimg.com/originals/f5/ef/a6/f5efa6a5b2c76c038ef0c8d2502fd2f6.jpg\" alt=\"[][][]\">", TestName = "AltWithMultipleBrackets")]


Картинка котика это клево, но это жестко выглядит в коде и явной пользы за собой не несет для теста, переноси строки, когда они вылезают за экран. Горизонтальный скролл - плохо, и больно для ребят с ноутбуками, у которых такие строчки 2 экрана бы занимали

Это и тестов ниже касается и одной переменной, которая с текстом написана в одну длинную строчку

Nevisinn · 2025-11-20T08:02:01Z

Честно - я не стал смотреть реализацию парсеров и детально смотреть реализацию парсера. Это сложно читать и разбираться, как и что работает.

Если кратко, то в парсерах лежит логика валидации токенов (точно ли этот токен заголовок, а не текст и т.д), и их преобразование

Одна из первых проблем этого решения - ParserContext, у нас относительно непредсказуемым образом он меняется то тут, то в парсерах. Каждый парсер еще и тип текущего токена может поменять в контексте, это прямо 100% нет, хотя бы потому, что мы точно можем этого избежать.

Если приходится менять тип текущего токена - значит токенайзер плохо выполнил свою работу. Добавь в него тогда нужную валидацию, а правда ли это токен или все же текст, посмотреть на соседние символы в токенайзере - норм. Считай что поток токенов к тебе приходит - иммутабельный, по нему можно только ходить, но не менять содержимое

Давай еще раз, в моем решении сейчас 3 этапа:

Токенизация без валидации
Парсинг токенов с валидацией и их преобразование.
Построение дерева из токенов

Правильно понимаю, что 1 и 2 этап должны слиться в один?

Возможно ты еще не совсем понял, зачем нам вообще этот подход нужен, можешь еще поискать в интернете, но основная идея такая:

Мы можем внутри парсинга одних токенов, вызывать парсинг других. Ну т.е. вместо 2 огромных свичей, сначала в выборе парсера, потом в BuildMarkdownDocument и сложной логики запрятанной в каждом из парсеров. Будет один парсер с методом ParseNode().

В ней уже будет свич, или например методы TryReadHeaderNode, TryReadItalicNode и т.п. подряд, если какой-то выдал true и смог спарсить - окей, и идем дальше форычем по всем токенам, пока они не кончатся.

В ParseNode мы строим дерево, верно? Без свича же не будет O(n)? Точно форычем, как тогда смещать указатель на токен?

Nevisinn · 2025-11-19T05:58:22Z

cs/Markdown/Tests/MdTests.cs

+    [TestCase(4000, TestName = "Parse_4000_repeats")]
+    [TestCase(8000, TestName = "Parse_8000_repeats")]
+    [TestCase(16000, TestName = "Parse_16000_repeats")]
+    public void Render_Performance_WithAllTokens(int repeatCount)


Тест на производительность должен автоматически проверять, на сколько время увеличилось в зависимости от N(длины строки) или такого варианта достаточно?

Идея теста правильная, реализация немного не та. Откажись от тесткейсов и сделай один тест, в котором также составляешь текст в N символов и в например 2*N символов. И потом внутри теста проверь, что первое число = второе число * 2, с учетом погрешности, разумеется

Yrwlcm · 2025-11-20T09:26:25Z

Токенизация без валидации

Парсинг токенов с валидацией и их преобразование.

Построение дерева из токенов

Правильно понимаю, что 1 и 2 этап должны слиться в один?

Да, мы не должны валидировать работу токенайзера. Он должен сразу работать правильно, обязанность одна, а разбили её на две составляющие

В ParseNode мы строим дерево, верно?

В целом да, только результирующие ноды ParseNode придется все равно сложить в какую-то родительскую ноду, потому что кажется не выйдет в ParseNode выдавать сразу полностью спаршенное дерево

Без свича же не будет O(n)?

Думаю получится и через свич реализовать в маркдауне, хотя не очень уверен. Но в целом да, я согласен что в худшем случае мы будем проходить какую-то часть токенов несколько раз. В общем случае, это наверное можно определить так, что в зависимости от того, сколько у нас будет конструкций языка, у которых префиксы совпадают, столько раз мы и будем повторятся по токенам. Ну условно private static string может быть началом свойства, а может быть началом метода (я не знаю как roslyn это на самом деле обрабатывает, но ради примера). Но такое мы через свич и не потянем без портянок обрабатывать. У нас же префиксы совпадают если не ошибаюсь только у _ и __ и это в итоге по сложности тоже должно быть линией или около того.

Отличие от твоего первого решения в том, что мы не проверяем CanParse, мы сразу парсим и если не получилось - откатываемся. А значит не обязательно заходим в каждый парсер, чтобы точно убедится, что он не может спарсить

Точно форычем, как тогда смещать указатель на токен?

Неа, тут я сказал не то, о чем думал, скорее while-ом пока не индекс за пределы списка токенов не выйдет

Построил предварительную архитектуру проекта

405f805

Nevisinn changed the title ~~Построил предварительную архитектуру проекта~~ Шубин Игорь Nov 5, 2025

Yrwlcm reviewed Nov 5, 2025

View reviewed changes

Обновил архитектуру проекта

a1b4f85

Yrwlcm reviewed Nov 8, 2025

View reviewed changes

Демо-версия

f7c1be6

Yrwlcm reviewed Nov 19, 2025

View reviewed changes

Nevisinn commented Nov 20, 2025

View reviewed changes

Шубин Игорь #263

Are you sure you want to change the base?

Шубин Игорь #263

Uh oh!

Conversation

Nevisinn commented Nov 5, 2025

Uh oh!

Yrwlcm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Yrwlcm Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Yrwlcm Nov 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Yrwlcm Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Yrwlcm Nov 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Yrwlcm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Nevisinn commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Yrwlcm commented Nov 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Yrwlcm Nov 5, 2025 •

edited

Loading

Yrwlcm Nov 8, 2025 •

edited

Loading

Yrwlcm Nov 5, 2025 •

edited

Loading

Yrwlcm Nov 8, 2025 •

edited

Loading

Nevisinn commented Nov 20, 2025 •

edited

Loading

Yrwlcm commented Nov 20, 2025 •

edited

Loading