-
Notifications
You must be signed in to change notification settings - Fork 300
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
Рушкова Ольга #225
base: master
Are you sure you want to change the base?
Рушкова Ольга #225
Conversation
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.
По моделькам вроде понятно(см. комментарии)
По сервисам и их взаимодействию пока не очень
cs/Markdown/TokenReader.cs
Outdated
{ | ||
public class TokenReader | ||
{ | ||
private string forRead; |
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.
Непонятно что это
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.
Подразумевалась строка. вдоль которой идёт Reader
cs/Markdown/Tags/Tag.cs
Outdated
{ | ||
throw new NotImplementedException(); | ||
} | ||
protected abstract bool AcceptWhileContextCorrect(char 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.
А как в этом случае будут отлавливаться пересекающиеся теги? Каждый тег будет содержать в себе логику, что он может содержать эти теги? Нерасширяемо
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.
+
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
html тег состоит из начала и конца
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.
Удалила из значения треугольные скобки в 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Значения нигде не задаются
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.
Добавила везде конструкторы
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А как он о тегах будет узнавать: как они выглядят, например? Похоже на протечку абстракций
cs/Markdown/Tags/Tag.cs
Outdated
protected abstract string mdTag { get; } | ||
protected abstract string htmlTag { get; } |
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.
Поля называются с большой буквы
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.
+
Add class structure