-
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
Implement Rustdoc #167
base: master
Are you sure you want to change the base?
Implement Rustdoc #167
Conversation
Fixes infinite recursion when we hit a specific case of an itertools iterator, a few visual glitches (too many or few line breaks), and adds the struct type distinction.
Some deficiencies mean we don't currently get all information. For example, qualifiers such as extern ABI and unsafe are dropped from methods and the information has_body is true even for provided methods.
Trait bounds and generics are still missing. Unifies the way that impls are scheduled such that we might group them soon.
They appear as ExternCrateItem 😂 That is obviously wrong and in particular this loses any information on the factual item name, on the crate origin, the path, and possibly more. We nevertheless at least try to render them here which works decently except for globs.
Also gates header levels to those that the backend actually supports. There is no subsubsubsubsection.
We had to make one slight adjustment in the order of Union and Struct, since they appear to have the same items now. This means that union can match union, with the type as an extra field that is ignored!
Uses the header adjustment strategy previously introduced to the code to ensure that scoping is correct. Most doc comments contain an h1 instead of the proper header level from the context. Note that in particular this will put some pressure on the backend to allow for slightly deeper nested headers, at least to 6. We already emulate them by a weighted font in one case.
@oberien I belive, as far as architectural changes are concerned, this is very much ready for a review. The basic set of features work although the style is anything but clean. Note the automatic header nesting on includes which is desirable for other includes as well. Current state translating ```config document_type = "article" lang = "en" titlepage = false title = "Test Markdown File" subtitle = "Showing heradoc's Features" author = "Me" date = "\\today" publisher = "My Publisher" advisor = "My Advisor" supervisor = "My Supervisor" citestyle = "ieee" header_includes = ["\\usepackage{lipsum}", "\\lstalias{text}{rust}", "\\lstalias{no_run}{rust}", "\\lstalias{compile_fail}{rust}"] geometry.margin = "2cm" ``` ![](rustdoc:///home/andreas/code/projects/image) |
It's important that this is the first step. This avoid any potential error that would come from trying to serialize a field that has an incompatible type, which would lead to the incorrect diagnosis that rust has output broken documentation.
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.
Quite a hefty PR. I really like it, as it also shows that our concept of decoupling the frontend, generator and backend somewhat works!
cfg: &'a Config, backend: impl Backend<'a>, arena: &'a Arena<String>, what_cargo: String, | ||
out: impl Write, stderr: Arc<Mutex<StandardStream>>, | ||
) -> FatalResult<()> { | ||
todo!() |
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 this intended, or is the PR still in a draft state?
.emit(); | ||
return Err(Fatal::Output(std::io::Error::new( | ||
std::io::ErrorKind::InvalidData, | ||
"Metadata call failed", |
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.
"Metadata call failed", | |
"couldn't get crate metadata from cargo", |
let krate = meta.workspace_members[0].split(' ').next().unwrap(); | ||
|
||
let format = Command::new("cargo") | ||
.args(&["+nightly", "rustdoc", "-p"]) |
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 nightly required? If yes, can we display a better error message if the user doesn't have nightly?
target.push({ | ||
// FIXME: support actually renamed library targets? | ||
let lib_name = format!("{}.json", krate); | ||
lib_name.replace("-", "_") | ||
}); |
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.
target.push({ | |
// FIXME: support actually renamed library targets? | |
let lib_name = format!("{}.json", krate); | |
lib_name.replace("-", "_") | |
}); | |
// FIXME: support actually renamed library targets? | |
target.push(format!("{}.json", krate).replace("-", "_")); |
Ok(krate) => Ok(krate), | ||
Err(err) => { | ||
diag | ||
.error("Cargo metadata failed for crate") |
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.
.error("Cargo metadata failed for crate") | |
.error("parsing rustdoc output failed for crate") |
FeEvent::ResolveInclude(_) | FeEvent::SyntheticInclude(..) => { | ||
unreachable!("ResolveInclude is handled by Generator") |
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 it makes sense to add a different error message for the different already-handled includes.
@@ -40,6 +41,7 @@ pub enum Event<'a> { | |||
InlineHtml(Cow<'a, str>), | |||
Latex(Cow<'a, str>), | |||
IncludeMarkdown(Box<Events<'a>>), | |||
IncludeRustdoc(Box<Rustdoc<'a>>), |
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.
Doing this as normal include is both crazy and amazing! I both love and hate it so much at the same time! :D
|
||
pub fn with_rustdoc(rustdoc: Rustdoc<'a>) -> Self { | ||
let frontend: Box<FeEvents<'a>> = Box::new(rustdoc.map(|event| { | ||
WithRange(event, (0..0).into()) |
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 it possible to add proper source and span information for diagnostics to be pointing to the original rust source code? I think we should open an issue for that as using the span 0..0 for everything feels weird.
frontend: Fuse<Frontend<'a>>, | ||
type FeEvents<'a> = dyn Iterator<Item=WithRange<FeEvent<'a>>> + 'a; | ||
|
||
pub struct MarkdownIter<'a> { |
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 a reason you're renaming this struct? So far I've followed having the different components in heradoc (frontend, generator, backend), and everyone that can be iterated over has its own Iter struct somewhere. Should it really be called "MarkdownIter"? It doesn't only iterate over markdown, but now also over rustdoc-output.
@@ -108,7 +108,7 @@ fn main() { | |||
} | |||
None => FileConfig::default(), | |||
}; | |||
let tmpdir = TempDir::new("heradoc").expect("can't create tempdir"); | |||
let tmpdir = std::mem::ManuallyDrop::new(TempDir::new("heradoc").expect("can't create tempdir")); |
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.
Where are we dropping the tempdir?
fn write_simple_header(name: &str, into: &mut impl Write, font: &str, level: i32) -> Result<()> { | ||
writeln!(into, "{}", "\\makeatletter")?; | ||
write!(into, r#"\newcommand\{}"#, name)?; | ||
write!(into, "{}{}{}{}", r#"{\@startsection{"#, name, "}{", level)?; |
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.
You can have {
/ }
in the result of format strings by using double {{
/ }}
like so:
println!("{{{}}}", 1337) // "{1337}"
Adds a new include type (as an url scheme
rustdoc
) that runscargo doc
on a target crate with json output then processes that document. Currently only supports local absolute paths.Example
rustdoc:crate
) resolves document relativerustdoc:/crate
) resolves project relativerustdoc:///path/crate
) resolves an absolute file pathInline documentation
Is rendered with the correct header level, starting with level 3. (The individual items from the documentation have level 2).
no_run
,compile_fail
,text
should be treated differently when appearing as the language annotation of a source code block.Source locations
The rustdoc output includes source files and span information, where convenient. We do not make use of it. The two possible future uses are:
a) somehow linking to it. This might be quite beneficial if we were to pack the source files into the output document via some embedding. (Polyglot as the PoC||GTFO journal does would be cool but is quite zip+pdf specific).
b) to refer to the item in the diagnostic output, when something fails.
Implementation progress
ExternCrate(currently bugged)TraitAlias(nightly feature),ForeignType(nightly feature, I guess),Primitive(ignored),Keyword(ignored),Maybe use the crate provided through: rust-lang/rust#81287
closes: #154