-
Notifications
You must be signed in to change notification settings - Fork 2
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
inline classes, #to_adoc, consistency renames #33
Conversation
- metanorma#31 added classes for inline elements - metanorma#10 added #to_adoc to Coradoc::Document classes based upon reverse_adoc code - renamed numbered/unnumbered to ordered/unordered for consistency with metanorma/asciidoc
anchor = @id ? "[[#{@id}]]\n" : "" | ||
title = ".#{@title}\n" unless @title.empty? | ||
"\n\n#{anchor}#{title}====\n" << lines << "\n====\n\n" | ||
end |
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.
Let's separate these into different classes. The number of ----
/ ___
etc. can change, e.g. 3, 4, 5, 6, 10. But always matching in begin/end.
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.
Moved to #41 .
lib/coradoc/document/inline/bold.rb
Outdated
end | ||
def to_adoc | ||
content = Coradoc::Generator.gen_adoc(@content) | ||
"#{content[/^\s*/]}*#{content.strip}*#{content[/\s*$/]}" |
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.
An inline bold element can be wrapped with *...*
or **...**
(doubled) in this case:
*bold* itself
attached**bold**here
Similarly for italics and monospace.
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.
Moved to #43.
end | ||
|
||
def level | ||
@level ||= level_from_string | ||
end | ||
|
||
def to_adoc | ||
content = Coradoc::Generator.gen_adoc(@content) |
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.
I want to differentiate @content
being String
content or Coradoc objects.
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.
differentiate in what way? @content can be both strong or Coradoc object because Coradoc::Generator.gen_adoc takes care of Coradoc objects, arrays, strings, nil
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.
I don't like the ambiguity of taking into different classes into this method. Can we use different methods for different types of input?
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.
Yes, goal would be for each method to be able to check if argument has one specific class?
About the class Title, now that I think about it, @content
should not be a string but TextElement instead. Or am I wrong and it can be just a string.
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.
It should be a TextElement, you’re right, because it can contain inline formatting like bold/italics and sup/sub etc.
opts = %{options="#{@options.join(',')}"} | ||
end | ||
|
||
[anchor, title, "video::", @src, "[", opts, "]"].join("") |
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.
I think we need to define the options.
Options need to be validated, e.g. height/width can be auto
or a pixel number. When given unknown options we should reject.
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.
Moved to #42.
Merging first. |
Coradoc::Document.to_adoc
method to output to AsciiDoc string #10 added #to_adoc to Coradoc::Document classes based upon reverse_adoc codeMetanorma PR checklist