Embedding the include at the bottom of the page#1753
Conversation
goldserg
force-pushed
the
merge-include-ch1
branch
from
88b12c3 to
7584bf1
Compare
goldserg
force-pushed
the
merge-include-ch1
branch
4 times, most recently
from
c638d63 to
992b77b
Compare
src/core/markdown/MarkdownService.ts
Outdated
| const isFileNotFound = | ||
| error instanceof Error && 'code' in error && error.code === 'ENOENT'; | ||
|
|
||
| if (isFileNotFound && from) { |
There was a problem hiding this comment.
The isFileNotFound && from condition is triggered for any include file in all build modes, not just when mergingIncludes. In regular md2html without pre-md2md, if the include file is actually missing from the disk, the error will be swallowed and the file will be silently rendered as empty. The user will receive a page with missing content without any error message?
src/commands/build/index.spec.ts
Outdated
| preprocess: { | ||
| hashIncludes: true, | ||
| mergeIncludes: false, | ||
| mergeIncludes: true, |
There was a problem hiding this comment.
shouldn't we consider this change to be breaking for other users of the open-source part of the cli? maybe we should enable this behavior with flags?
There was a problem hiding this comment.
It's only for test. I change this before merge. And set true, after part 2
| depContent = addIndent(depContent, indent); | ||
| } | ||
|
|
||
| // Add source map comment at the beginning of inlined content |
There was a problem hiding this comment.
Yes, I test the sourcemaps in our documentation.
| const appendix = '\n' + blocks.join('\n'); | ||
| scheduler.add([0, 0], async (content) => content + appendix, {}); | ||
| } | ||
| } as StepFunction; |
There was a problem hiding this comment.
The file has more than 500 lines. Can we split it into more readable parts?
There was a problem hiding this comment.
I've been thinking about it, and it's going to get a little bigger when the terms are processed. I think I'll break it in the second final PR
src/core/markdown/MarkdownService.ts
Outdated
| } catch (error) { | ||
| const isMergedInput = | ||
| this.config.outputFormat !== 'md' && this.config.preprocess?.mergeIncludes; | ||
| if (isMergedInput) { |
There was a problem hiding this comment.
In merge-includes mode, any error is still caught, not just ENOENT. For example: the file exists, but there are no read permissions, and the error will be swallowed.
} catch (error) {
...
const isFileNotFound =
error instanceof Error && 'code' in error && error.code === 'ENOENT';
if (isMergedInput && isFileNotFound) {
return [];
}
throw error;
}
chore: linting, tests, sonar chore: sonar
chore: refactoring chore: refactoring chore: refactoring chore: fix after review
chore: fix by review
goldserg
force-pushed
the
merge-include-ch1
branch
from
0ef18c5 to
13d26f7
Compare
|
2 participants
No description provided.