Skip to content
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

Савенко Дмитрий #228

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Asrom11
Copy link

@Asrom11 Asrom11 commented Nov 24, 2024


namespace MarkDownTest;

public class MarkdownParserTests
Copy link

Choose a reason for hiding this comment

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

Не хватает тестов

  • На экранирование различных управляющих символов (в том числе самого \)
  • Когда переносы строк разрывают управляющие символы

Copy link

Choose a reason for hiding this comment

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

Кажется не работает требование В то же время выделение в ра_зных сл_овах не работает.

Надеюсь правильно написал тест))

    [Test]
    public void TestWords()
    {
        var input = "ра_зных сл_овах";
        var expectedTokens = new[]
        {
            Token.CreateText("ра_зных сл_овах", 0)
        };
        
        var tokens = _parser.Parse(input);
        
        tokens.Should().BeEquivalentTo(expectedTokens, 
            options => options.WithStrictOrdering());
    }

Copy link

Choose a reason for hiding this comment

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

И ещё неработающее требование За подчерками, начинающими выделение, должен следовать непробельный символ. Иначе эти_ подчерки_ не считаются выделением и остаются просто символами подчерка.

    [Test]
    public void TestWords()
    {
        var input = "эти_ подчерки_ не должны работать";
        var expectedTokens = new[]
        {
            Token.CreateText("эти_ подчерки_ не должны работать", 0)
        };
        
        var tokens = _parser.Parse(input);
        
        tokens.Should().BeEquivalentTo(expectedTokens, 
            options => options.WithStrictOrdering());
    }

Copy link

Choose a reason for hiding this comment

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

И ещё Подчерки, заканчивающие выделение, должны следовать за непробельным символом. Иначе эти _подчерки _не считаются_ окончанием выделения и остаются просто символами подчерка.

    [Test]
    public void TestWords()
    {
        var input = "_подчерки _не считаются_";
        var expectedTokens = new[]
        {
            Token.CreateText("_подчерки ", 0),
            Token.CreateItalic(true, 10),
            Token.CreateText("не считаются", 11),
            Token.CreateItalic(true, 23)
        };
        
        var tokens = _parser.Parse(input);
        
        tokens.Should().BeEquivalentTo(expectedTokens, 
            options => options.WithStrictOrdering());
    }

Copy link

Choose a reason for hiding this comment

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

И ещё __Непарные_ символы в рамках одного абзаца не считаются выделением.

    [Test]
    public void TestWords()
    {
        var input = "__Непарные_ символы";
        var expectedTokens = new[]
        {
            Token.CreateText("__Непарные_ символы", 0)
        };
        
        var tokens = _parser.Parse(input);
        
        tokens.Should().BeEquivalentTo(expectedTokens, 
            options => options.WithStrictOrdering());
    }

Copy link

Choose a reason for hiding this comment

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

И ещё Подчерки внутри текста c цифрами_12_3 не считаются выделением и должны оставаться символами подчерка.

    [Test]
    public void TestWords()
    {
        var input = "цифрами_12_3";
        var expectedTokens = new[]
        {
            Token.CreateText("цифрами_12_3", 0)
        };
        
        var tokens = _parser.Parse(input);
        
        tokens.Should().BeEquivalentTo(expectedTokens, 
            options => options.WithStrictOrdering());
    }

Copy link

Choose a reason for hiding this comment

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

И ещё Если внутри подчерков пустая строка ____, то они остаются символами подчерка.

    [Test]
    public void TestWords()
    {
        var input = "____";
        var expectedTokens = new[]
        {
            Token.CreateText("____", 0)
        };
        
        var tokens = _parser.Parse(input);
        
        tokens.Should().BeEquivalentTo(expectedTokens, 
            options => options.WithStrictOrdering());
    }

Comment on lines 30 to 31
tokens.Should().BeEquivalentTo(expectedTokens,
options => options.WithStrictOrdering());
Copy link

Choose a reason for hiding this comment

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

Во всех тестах одна и та же проверка в конце теста со нестандартными настройками, можно вынести в метод и заюзать везде

}

[Test]
public void Parse_NestedTags_HandlesNestedStrongAndItalic()
Copy link

Choose a reason for hiding this comment

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

Тест полностью дублирует Parse_NestedEmphasis_ReturnsCorrectTokens, наверное хотел жирный и курсив местами поменять в одном из тестов?

Comment on lines +202 to +203
[TestCase("")]
[TestCase(" ")]
Copy link

Choose a reason for hiding this comment

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

По классике, там где пустая строка и пробел, должен и null быть)

if (closingBracketIndex == -1 || closingBracketIndex + 1 >= text.Length || text[closingBracketIndex + 1] != '(')
return false;

var closingParenIndex = text.IndexOf(')', closingBracketIndex + 1);
Copy link

Choose a reason for hiding this comment

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

closingParentIndex

return false;

var level = 1;
var pos = context.Position + 1;
Copy link

Choose a reason for hiding this comment

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

position

Comment on lines +5 to +14
public string Opening { get; }
public string Closing { get; }
public TokenType Type { get; }

public Delimiter(string opening, string closing, TokenType type)
{
Opening = opening;
Closing = closing;
Type = type;
}
Copy link

Choose a reason for hiding this comment

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

Не обязательно внедрять сейчас, но если захочешь, вдруг понравится

public class Delimiter
{
    public required string Opening { get; init; }
    public required string Closing { get; init; }
    public required TokenType Type { get; init; }
}

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

Link
}

public enum TagState
Copy link

Choose a reason for hiding this comment

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

Лучше в отдельный файлик

Comment on lines 15 to 32
switch (token.Type)
{
case TokenType.Text:
result.Append(System.Net.WebUtility.HtmlEncode(token.Text));
break;

case TokenType.Strong:
case TokenType.Italic:
HandleFormattingTag(token, tagStack, result);
break;

case TokenType.Header:
HandleHeaderTag(token, tagStack, result);
break;
case TokenType.Link:
HandleLinkToken(token, result);
break;
}
Copy link

Choose a reason for hiding this comment

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants