Рушкова Ольга #225
Рушкова Ольга #225
Conversation
cs/Markdown/TokenReader.cs
Outdated
| { | ||
| public class TokenReader | ||
| { | ||
| private string forRead; |
There was a problem hiding this comment.
Подразумевалась строка. вдоль которой идёт Reader
cs/Markdown/Tags/Tag.cs
Outdated
| { | ||
| throw new NotImplementedException(); | ||
| } | ||
| protected abstract bool AcceptWhileContextCorrect(char current); |
There was a problem hiding this comment.
А как в этом случае будут отлавливаться пересекающиеся теги? Каждый тег будет содержать в себе логику, что он может содержать эти теги? Нерасширяемо
cs/Markdown/Tags/Tag.cs
Outdated
| { | ||
| protected Token context; | ||
| protected abstract string mdTag { get; } | ||
| protected abstract string htmlTag { get; } |
There was a problem hiding this comment.
html тег состоит из начала и конца
There was a problem hiding this comment.
Удалила из значения треугольные скобки в RenderHtml уже буду посдставлять что-то вроде $"<{htmlTag}>{внутренняя строка}<\{htmlTag}>"
cs/Markdown/Token.cs
Outdated
| public int Position { get; } | ||
| public int Length { get; } | ||
|
|
||
| private string source; |
There was a problem hiding this comment.
Добавила везде конструкторы
cs/Markdown/TokenReader.cs
Outdated
| private string forRead; | ||
| public int Position { get; } | ||
|
|
||
| public Token ReadWhile(Func<char, bool> accept) |
There was a problem hiding this comment.
А как он о тегах будет узнавать: как они выглядят, например? Похоже на протечку абстракций
cs/Markdown/Tags/Tag.cs
Outdated
| protected abstract string mdTag { get; } | ||
| protected abstract string htmlTag { get; } |
There was a problem hiding this comment.
Поля называются с большой буквы
cs/clean-code.sln
Outdated
| @@ -1,4 +1,4 @@ | |||
| | |||
| | |||
There was a problem hiding this comment.
А почему в решение не добавлены проекты?
cs/Markdown/Tags/Tag.cs
Outdated
| { | ||
| public bool IsTagClosed { get; protected set; } | ||
| private Token? context; | ||
| public int TagStart = tagStart; |
There was a problem hiding this comment.
А зачем? У тебя же есть поле tagStart. созданное изначально?
cs/Markdown/Tags/Italic.cs
Outdated
| protected Italic(string mdText, int tagStart) : base(mdText, tagStart) | ||
| { | ||
| MarkdownText = mdText; | ||
| TagStart = tagStart; | ||
| } | ||
|
|
||
| public static Italic CreateInstance(string markdownText, int tagStart) | ||
| { | ||
| if (tagStart > 0 && char.IsLetter(markdownText[tagStart - 1])) | ||
| return new ItalicSelectPartOfOneWord(markdownText, tagStart); | ||
|
|
||
| return new ItalicSelectFewWords(markdownText, tagStart); | ||
| } |
There was a problem hiding this comment.
Чет странное. Кмк лучше выделить отдельно фабрику для тегов, если они чем-то отличаются, а не два создания класса в один пихать
cs/Markdown/MdParser.cs
Outdated
|
|
||
| private Tag CreateTag(ReaderPosition current, List<Tag> external) | ||
| { | ||
| var openTag = Md.GetOpenTag(current.Position, markdownText, out int contextStart); |
There was a problem hiding this comment.
Лапша получается. Класс Md создает MdParser, MdParser обращается к Md.
Желаемая архитектура все же когда у тебя нижележащие сервисы не только не обращаются к вышележащим, но и ничего про них не знают вообще
cs/Markdown/Md.cs
Outdated
| var tag = tagBegin == '_' && tagStart != markdownText.Length - 1 && markdownText[tagStart + 1] == '_' | ||
| ? "__" | ||
| : tagBegin.ToString(); | ||
| contextStart = tagStart + tag.Length; | ||
| return tags[tag](markdownText, tagStart); |
There was a problem hiding this comment.
Общий метод знает слишком много про спецификацию, будет сложно расширять, нужно помнить о том, что здесь есть логика именно про _ и __
cs/Markdown/Token.cs
Outdated
|
|
||
| public string GetValue() | ||
| { | ||
| return new string(source.Substring(Position, Length)); |
There was a problem hiding this comment.
Зачем создавать новую строки из строки? У тебя Substring уже строчку возвращает
| && (MarkdownText.Substring(currentPosition, MdTagClose.Length) == MdTagClose); | ||
| } | ||
|
|
||
| public override Type GetType() => typeof(Bold); |
There was a problem hiding this comment.
Зачем? typeof(object?) не подходит?
cs/Markdown/MdRulesForNestedTags.cs
Outdated
| {typeof(Header), new () { typeof(Bold), typeof(Italic), typeof(Escape)}}, | ||
| {typeof(Bold), new () { typeof(Italic), typeof(Escape)}}, | ||
| {typeof(Italic), new () { typeof(Escape)}} |
There was a problem hiding this comment.
По типам сохранять - плохая практика. Введи enum и соответствующее поле, если хочешь использовать в таком виде. Через наследование ты сможешь его везде затребовать.
Еще не очень нравится, что это хранится в оторванности от самих тегов.
Почему это не хранить в соответствующем тэге?
| { | ||
| var rules = new MdRulesForNestedTags(); | ||
|
|
||
| var result = rules.IsNestedTagWorks(Bold.CreateInstance("", 0), Italic.CreateInstance("", 0)); |
There was a problem hiding this comment.
Даже тут видно, что странное происходит:
Тебе нужно создать тег с текстом, позицией и тд, чтобы просто проверить является ли один тег вложенным для другого
|
|
||
| [TestCaseSource(nameof(NestedTagsTestCases))] | ||
| [TestCaseSource(nameof(IncorrectTagsTests))] | ||
| public void Render_Should(string mdText, string expectedTagsToHtml) |
There was a problem hiding this comment.
Неправильно превращается текст, где больше 1 MD-тега:
"__FirstTag__, __SecondTag__" -> "FirstTag__, __SecondTag"
"__AaAa__, _b_" -> "__AaAa__, _b_"
Не превращается парный тег '_':
"__tag_" ->"__tag_"
Однако выделять часть слова они могут: и в нач_але, и в сер_еди_не, и в кон_це. - Не работает
н_ачал_о -> н_ачал_о
Если внутри подчерков пустая строка ____, то они остаются символами подчерка.
\n -> \n
При экранировании рядом с одним символов ломается тег:
_вот это будет \д_ выделено тегом_ -> <em>вот это будет \д_ выделено тегом</em>
Add class structure