-
Notifications
You must be signed in to change notification settings - Fork 319
Шипицын Павел #258
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Шипицын Павел #258
Conversation
|
|
||
| namespace Markdown.TokenRenders; | ||
|
|
||
| public class HeaderRender : ITokenRender |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Не реализуют интерфейс в всех рендерах
| public class Token | ||
| { | ||
| public TokenType Type { get; init; } | ||
| public string Content { get; set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Почему значение токена может изменяться? Разбиение не токены должно производиться единично, а уже потом значение как-то обрабатываться
| { | ||
| public TokenType Type { get; init; } | ||
| public string Content { get; set; } | ||
| public List<Token> Children { get; init; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Опять же изменяемое свойство, из любого места в коде я могу удалить какие-то токены из списка, что небезопасно и приводит к усложнению кода. Корректней было бы использовать IReadOnlyList.
| using Markdown.Interfaces; | ||
| using Markdown.Parsers; | ||
|
|
||
| namespace Markdown.Handlers; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Неймспейс неправильный
| public class EscapeHandler : ITokenHandler | ||
| { | ||
| public bool CanHandle(char current, char next, ParserContext context) | ||
| => current == '\\'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Завязка на константу. Если захотим поменять знак экранирования на другой, к примеру на %, то тебе придётся во всех местах искать это значение захардкоженное. Нужно вынести это куда-то, желательно так чтобы автоматически обновлять в нужных местах
| context.CurrentIndex += Delimiter.Length; | ||
| } | ||
|
|
||
| private static int FindSingleDelimiter(string text, int startIndex, int paragraphEndIndex, string delimiter) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Очень сложная операция, которая вызывается ещё и несколько раз за один проход
| } | ||
|
|
||
| protected abstract bool HasValidNesting(ParserContext context); | ||
| private bool IsValidBoundary(ParserContext context) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Очень сложно разобраться в том что тут написано, я не буду даже пытаться это сделать
|
|
||
| if (context.Stack.Count > 0) | ||
| { | ||
| if (context.Stack.Peek().Type == TokenType.Italics && TokenType == TokenType.Strong) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А если ещё какие-то пересечения захотим проверять, надо каждый раз условие новое добавлять
| { | ||
| var index = context.CurrentIndex; | ||
| var text = context.MarkdownText; | ||
| var nextDelimiter = TokenType == TokenType.Strong ? "_" : "__"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Константы
| char.IsLetterOrDigit(text[closingIndex + Delimiter.Length])); | ||
| if (isInsideWord) | ||
| { | ||
| if (index > 0 && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Нечитаемая конструкция, нужно упростить
@tripples25
Основная идея заключается в том чтобы преобразовать входной тест в абстрактное иерархическое перечисление токенов, по сути синтаксическое дерево, а затем преобразовать это дерево токенов в Html строку. Так можно будет разделить ответственность за разбор исходного текста и построение результирующей строки, а также легче будет добавить новые теги.
Основные классы:
Tokenпредставляет абстрактные элементы разметки и хранит содержимое и вложенные токены.MarkdownParserразбирает текст, используя различныеTokenHandler-ы для разных токенов и контекст (ParserContext) для управления состоянием.HTMLRenderпреобразует токены в HTML-строку используяTokenRender-ы для преобразования конкретного токена в HTML тег.Пока успел реализовать только MarkdownParser