-
Notifications
You must be signed in to change notification settings - Fork 1
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
Update dokka to 1.4 #23
Conversation
dd54fbc
to
7b53d0d
Compare
// replace title with original markdown file name | ||
val original = super.pathTo(node, context) | ||
val paths = original.split("/") | ||
(paths.dropLast(1) + listOf(node.template.template.name())).joinToString("/") |
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.
this (template.template
) naming is a bit strange and confusing, what about naming it loadedTemplate
and templateFile
respectively?
import java.net.URL | ||
|
||
|
||
// Thisi is a code taken from dokka, will be removed after PR with fixes is merged into dokka |
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.
Can we have a link to the pr / pr number here?
protected fun File.asDri(): DRI { | ||
val relativePath = root.toPath().relativize(toPath()).toString().replace(File.separatorChar, '.') | ||
return DRI("_.$relativePath") | ||
data class LoadedTemplete(val template: TemplateFile, val children: List<LoadedTemplete>, val file: File) { |
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.
s/LoadedTemplete/LoadedTemplate/
and maybe rename the val template
to avoid confusion?
emptySet(), | ||
PropertyContainer.empty() | ||
) | ||
return PartiallyRenderedContend( |
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.
s/Contend/Content/
|
||
val driMap = all.flatMap { flatten(it) }.map { it to pathToDri(it) }.toMap() | ||
|
||
fun templeteToPage(myTemplate: LoadedTemplete): StaticPageNode { |
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.
s/templeteToPage/templateToPage/
import org.jetbrains.dokka.plugability.DokkaContext | ||
import java.net.URI | ||
|
||
class ExternalDocsToolRenderer(context: DokkaContext) : org.jetbrains.dokka.base.renderers.html.HtmlRenderer(context) { | ||
|
||
// this will be reweitten after PR is merged to dokka |
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.
is there any link to the PR or its number?
|
||
override val content: ContentNode = PreRenderedContent(resolved.code, DCI(dri, ContentKind.Empty), mySourceSet) | ||
fun title(): String = template.template.title() | ||
fun noFrame(): Boolean = template.template.noFrame() |
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 is okay and I am not orthodox about negations but just for everyone's information:
https://schneide.blog/2014/08/03/dont-ever-not-avoid-negative-logic/
So it could be hasFrame
but noFrame
works too to me
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.
That is a good point.
.gitignore
Outdated
@@ -4,3 +4,4 @@ | |||
build/ | |||
gradle/ | |||
lib/ | |||
out |
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 is a good practice to add the new lines at the end of file
https://stackoverflow.com/questions/729692/why-should-text-files-end-with-a-newline
HtmlRenderer.builder(ctx.markdownOptions).build().render(parser.parse(rendered)) | ||
} | ||
val resources = listSetting("extraCSS") + listSetting("extraJS") | ||
layoutTemplate?.let { |
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.
redundant let call
Another thing is that the relative links do not work in real life (comparing to tests) yet. Try to reference some |
Relative links does not work, still but we got an issue to fix it #17 |
76da626
to
ae4fbcb
Compare
Change the way how we render pages, so we parse .md files and then apply html layouts. Do no render files other then .md or .html ones
ae4fbcb
to
4a36cc6
Compare
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.
lgtm then
Change the way how we render pages, so we parse .md files and then apply html layouts.
Add basic support for linking.