Stream: rustdoc

Topic: extend source code viewer


view this post on Zulip GuillaumeGomez (Apr 01 2021 at 18:44):

I intend to extend a bit the source code viewer by turning functions and types used into links to their definition. So if you have:

let x: String = "12".to_owned();

String would link to the String definition. As a second step, it'd be awesome to be able to generate link for to_owned too but let's do it one step at a time. Depending on how we implement it, it could be very trivial though.

I took a quick look to see how we do currently but we use something close to what we have in proc macro, meaning we only have tokens. I looked around a bit but I don't think we can extract much information just from a Span and the TyCtxt (unless I missed it) so I plan to keep a copy of the original AST we get and then use it alongside the tokens so that I can get the item from its span.

So I opened this topic mostly to see with you if that sounds like a valid approach to you or if maybe you have some other ideas?

view this post on Zulip Joshua Nelson (Apr 01 2021 at 19:02):

I don't think you can do this with only the AST, you at least need the Res of the type

view this post on Zulip Joshua Nelson (Apr 01 2021 at 19:04):

in general to link type-relative paths you'll need at least the HIR. To link to_owned() you'll need type information which will break https://doc.rust-lang.org/nightly/rustdoc/advanced-features.html#cfgdoc-documenting-platform-specific-or-feature-specific-information. So you'll need to ignore errors and let the links go if they cause an error.

view this post on Zulip Manish Goregaokar (Apr 01 2021 at 21:58):

I think this is a major expansion of scope and should not be taken lightly. I've wanted this for a while as well; but you need a _lot_ more analysis to do this right. It will also bloat the src sizes a ton.

Note that there already exists a tool that does this: https://github.com/rust-dev-tools/cargo-src . We should consider putting effort into that _first_, and then perhaps making that a dependency of rustdoc. I'm very wary of making it a part of rustdoc.

view this post on Zulip Noah Lev (Apr 01 2021 at 23:22):

Also, if we end up implementing this, I think we should keep the UI the same (i.e., no link underlines) so as not to make the output busy.

view this post on Zulip GuillaumeGomez (Apr 02 2021 at 08:27):

Camelid said:

Also, if we end up implementing this, I think we should keep the UI the same (i.e., no link underlines) so as not to make the output busy.

Agreed. I intended to just updated when you go over it like we do in the item path at the top of the items' page.

@Manish Goregaokar This is why I wanted to give a try: to be able to actually see the difference both in file size and in rendering performance. With all this information, we can see exactly the impact.

view this post on Zulip Manish Goregaokar (Apr 02 2021 at 08:28):

Seems fair. My understanding is that cargo src is a heavy piece of software, and from what I know from clippy this stuff is pretty heavy, so worth experimenting but I think cargo-src is a good place to put effort into a reusable library that rustdoc can then use

view this post on Zulip GuillaumeGomez (Apr 02 2021 at 08:41):

My understanding is that cargo-src is going through all code once again. If we could re-use the information rustdoc has directly, we might be able to cut down the run time. But that could be interesting to see that directly with nrc. Well, first, I'll start reading cargo-src source code and see how they do.

view this post on Zulip GuillaumeGomez (Apr 02 2021 at 11:48):

Actually no: cargo-src is relying on rls, so cannot be used inside rustdoc...

view this post on Zulip simulacrum (Apr 02 2021 at 13:45):

FWIW I've had pretty good success in some research work I've been doing with running rust-analyzer as a subprocess and doing go-to-def queries to recover information from an AST, and that is actually really fast overall

view this post on Zulip GuillaumeGomez (Apr 02 2021 at 13:47):

Someone just suggested me to look at the rust-analyzer source code. If it can be used as a library, that'd make this whole thing muuuuch simpler

view this post on Zulip simulacrum (Apr 02 2021 at 13:54):

Manish Goregaokar said:

I think this is a major expansion of scope and should not be taken lightly. I've wanted this for a while as well; but you need a _lot_ more analysis to do this right. It will also bloat the src sizes a ton.

Note that there already exists a tool that does this: https://github.com/rust-dev-tools/cargo-src . We should consider putting effort into that _first_, and then perhaps making that a dependency of rustdoc. I'm very wary of making it a part of rustdoc.

I am also unconvinced FWIW that this is a good fit for rustdoc's src view; it feels like a pretty large expansion in scope and may be better suited for an alternative tool. I'm wary of pushing too much static analysis into rustdoc, personally.

view this post on Zulip simulacrum (Apr 02 2021 at 13:55):

GuillaumeGomez said:

Someone just suggested me to look at the rust-analyzer source code. If it can be used as a library, that'd make this whole thing muuuuch simpler

The LSP protocol is not too hard to get started with, fwiw, I don't think using it as a binary over that would cause many problems for prototyping

view this post on Zulip simulacrum (Apr 02 2021 at 13:55):

I managed to get goto-def with ~100 lines of code or so

view this post on Zulip simulacrum (Apr 02 2021 at 13:56):

(Can put up a gist of that file if useful)

view this post on Zulip GuillaumeGomez (Apr 02 2021 at 14:05):

I was hoping to not run the server part and simply use their library to build the map I needed

view this post on Zulip simulacrum (Apr 02 2021 at 14:09):

I mean, I hear you, but for experimentation I don't think the server being around is really that bad, and gets you up and running much faster :)

view this post on Zulip GuillaumeGomez (Apr 02 2021 at 14:10):

then sure, I'd gladly like to have that gist ;)

view this post on Zulip simulacrum (Apr 02 2021 at 14:17):

@GuillaumeGomez https://gist.github.com/Mark-Simulacrum/c173aa652d1e327358c9279120a9896c

view this post on Zulip GuillaumeGomez (Apr 02 2021 at 14:17):

Thanks!

view this post on Zulip simulacrum (Apr 02 2021 at 14:18):

that has the constants defined in a sort of hacky way for my use case (that particular one is off of a c file) but it shouldn't be hard to alter them for rust etc

view this post on Zulip simulacrum (Apr 02 2021 at 14:18):

(e.g., I think that invokes clangd and not rust-analyzer)

view this post on Zulip simulacrum (Apr 02 2021 at 14:18):

but it should work with minor adjustments for rust-analyzer too, happy to help with questions as well

view this post on Zulip GuillaumeGomez (Apr 02 2021 at 14:18):

I'll alter the code as needed then ;)

view this post on Zulip GuillaumeGomez (Apr 02 2021 at 14:19):

but first, a break

view this post on Zulip GuillaumeGomez (Apr 02 2021 at 14:19):

thanks again!

view this post on Zulip Manish Goregaokar (Apr 06 2021 at 22:13):

an idea I posted elsewhere which may be relevant is that I think a good way forward here is to make cargo-src really good (and maybe make it possible for it to output static html pages), and then give rustdoc a config to change its src links to something else. this can also be useful for having src link to github

view this post on Zulip Manish Goregaokar (Apr 06 2021 at 22:14):

and we can work on shipping cargo-src, and having tigher integration, but we'd have a separate cargo-src team

view this post on Zulip Manish Goregaokar (Apr 06 2021 at 22:14):

and they would commit to building that tool as is

view this post on Zulip Manish Goregaokar (Apr 06 2021 at 22:14):

because adding this in rustdoc is a _huge_ expansion of scope

view this post on Zulip Joshua Nelson (Apr 06 2021 at 22:17):

Manish Goregaokar said:

and we can work on shipping cargo-src, and having tigher integration, but we'd have a separate cargo-src team

is there a cargo-src team btw? I haven't seen activity in the discord channel since december 2019 :sweat_smile:

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 14:41):

There is not; it was kinda built as a proof of concept for RLS

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:09):

I clarified this with nox on twitter but it's worth clarifying here too: I don't mean to say that "cargo-src exists so we shouldn't do this", or that we should add it to cargo-src: cargo-src has problems too; it is server-based (unlike rustdoc, which is files-based), and uses RLS save-analysis stuff, and doesn't currently work.

I mostly mean that I would like to see this in a separate tool _like_ cargo-src. Docs.rs can use it by default, and we can make it easy for others to use by default, but I'd rather have a first-class separately-maintained tool for this.

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:11):

This could even be a pluggable "source generation" library

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:11):

so rustdoc's current source library just generates sources, but you can plug in this library and get better stuff

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:11):

and there's a way to use it separately

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:12):

ooh like pretty_assert

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:12):

I like that

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:14):

cc @GuillaumeGomez instead of discussing this over twitter

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:15):

I don't see the point of having that done in a separate library. :-/

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:16):

As for twitter, I'm simply providing info there too. ;)

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:17):

But just for here to have as much information: this is my WIP: https://github.com/rust-lang/rust/compare/master...GuillaumeGomez:src-to-definition?expand=1

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:17):

as you can see, it's really small

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:17):

it doesn't try to do much and I don't think we should do much more than that

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:23):

@GuillaumeGomez I think this prototype is pretty small and i would be fine with maintaining just this functionality; i'm not convinced that a _good_ version of this feature will be at all this simple

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:23):

my argument in favor of that is "look how big cargo-src is"

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:24):

No, cargo-src is handling cross-crates and a lot more things like providing documentation directly when you hover something

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:24):

yeah

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:25):

why shouldn't we handle cross-crate?

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:25):

that's kinda my point, cross-crate seems like crucial functionality here?

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:25):

(might be misunderstanding cross-crate)

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:25):

well, for example I didn't plan to allow to provide link on String in things outside std

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:26):

okay, this is exactly my point :)

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:26):

i'm not convinced that a _good_ version of this feature will be at all this simple

I think cross-crate is an example of what people will expect for a good version of this

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:26):

Well, good version can come a loooot later

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:26):

my argument is not one of implementation effort

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:27):

for now we can simply have it on nightly and that's it

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:27):

oh, my bad, please explain then

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:32):

so taking a step back this is what i mean by expansion of scope, too: right now, [src] is bare minimum, and people expect it to be bare minimum. people do not ask for features to make it _the absolutely best perfect src viewers_. right? i mean, there are feature requests, but not huge ones.

This is fine, this isn't great; but as a team we focus on maintaining certain areas of rustdoc. we don't do much with src, we also don't need to spend time on it. great!

the moment we try to make src do lots of cool things that you _cannot find otherwise_, that becomes a primary focus of the tool. rustdoc becomes the tool for generating crosslinked src. which is fine, but it also means that people will want it to be _good_, and this means it becomes very dumb for us to _not_ support things like cross-crate :)

remember that intra doc links also went through an rfc, and it also took a while for it to get things like cross-crate and such. it's a significant portion of our maintenance burden now, which is _fine_, but the team agreed to maintain that

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:32):

(in meetings)

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:35):

Yes and it's handled correctly. If your main argument is the maintenance burden, then I don't see the issue. People want more out of the source code viewer as the tweet reactions showed. I don't see any issue with implementing it

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:36):

I think with an RFC we can see what people would want in such a thing and then carefully define our scope

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:37):

because it's quite possible they'll want things like "link every identifier to its type" (HUGE amount of load)

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:38):

I don't think an RFC is that big a barrier here

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:38):

But an RFC would prevent us to do anything until then. What I suggest is instead to have something small only on nightly and when we have something, we then write an RFC to stabilize it

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:38):

it definitely is

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:38):

@GuillaumeGomez oh i'm happy to allow this to land on nightly behind a cli flag

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:38):

GuillaumeGomez said:

But an RFC would prevent us to do anything until then. What I suggest is instead to have something small only on nightly and when we have something, we then write an RFC to stabilize it

I am strongly against this because docs.rs uses nightly

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:38):

the second people can use it they will, and they'll be mad if it's removed

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:38):

@Joshua Nelson okay behind a cli flag that docs.rs doesn't use?

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:38):

Manish Goregaokar said:

Joshua Nelson okay behind a cli flag that docs.rs doesn't use?

I would still rather not because you can pass custom rustflags

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:39):

@Joshua Nelson Should have precised a nightly argument, my bad

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:39):

like, I'm ok if we get consensus that we want "something like this" and then put it on nightly behind a feature gate

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:39):

but I don't think we should implement it before we decide whether we want it at all

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:39):

@Joshua Nelson oh yeah feature gate is fine

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:40):

I feel like I'm not explaining this well :/

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:40):

the second people can use it they will, and they'll be mad if it's removed
we kinda made this mistake with intra-doc, though we didn't really want to remove it

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:40):

@Joshua Nelson from twitter, it seems quite unanimous. In any case, my PR is far from done so we have plenty of time. And until it's stabilized, we can completely change it as many times as we want

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:40):

my concern is that people will use it as if it's stable whether it has a feature gate or not, look at all the people going to https://github.com/rust-lang/rust/issues/82768 saying they want to use it on docs.rs immediately

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:41):

And until it's stabilized, we can completely change it as many times as we want

that's my point though, if people start using it immediately we can't really :/

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:41):

I can understand them haha

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:41):

No, if it's nightly, it's prone to change

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:41):

that's the deal

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:41):

it's like a preview feature: "use it at your own risk"

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:41):

maybe the solution here is docs.rs shouldn't use nightly

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:42):

@Joshua Nelson it needs to because some crates are nightly only

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:42):

I don't see any other way for stability guarentees to actually mean something

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:43):

@GuillaumeGomez i said this in the case of notable_trait, but you've gotta be careful making conclusions off of twitter/etc like that because the conclusion you can make from your tweet is that folks want this functionality _somewhere_.

I'm totally fine with this feature being something rustdoc can hook into (even by default!). I'm just wary of maintaining it inside rustdoc _without understanding the scope_. An RFC helps us understand the scope

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:44):

and @Joshua Nelson's point beautifully illustrates why product-oriented thinking is important here, in that analogy docs.rs is our consumer and we need to make sure they can deal

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:45):

nightly features are prone to breaking change, so i don't see how it comes in scope here

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:45):

I agree for the RFC, but only once we have tested the feature for some time and got feedback so we know what is expected

view this post on Zulip CraftSpider (Apr 07 2021 at 15:45):

Just to chime in, I personally like the sound of the 'pluggable' approach Manish mentioned.

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:46):

GuillaumeGomez said:

I agree for the RFC, but only once we have tested the feature for some time and got feedback so we know what is expected

this is what experimental RFCs are for

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:46):

so you don't have to lock down the final product, you're just exploring the space

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:46):

I really don't like the idea of adding this without at least an eRFC

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:47):

CraftSpider said:

Just to chime in, I personally like the sound of the 'pluggable' approach Manish mentioned.

I'm curious: why?

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:47):

would it make sense to have the intra-doc links as a plugin too?

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:47):

and themes too?

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:48):

themes are a plugin

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:49):

you can pass --theme on stable

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:49):

No, they are not

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:49):

It's an option

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:49):

there are 3 by default and you can extend them

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:49):

themes have an "API" that is the "stable" css classes

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:49):

GuillaumeGomez said:

there are 3 by default and you can extend them

that is literally what a plugin is lol

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:49):

@GuillaumeGomez curious: when we say pluggable, are you assuming that we're declaring it won't be the default?

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:49):

because that's not what we're saying

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:49):

Ah, maybe we have a different meaning for plugin

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:50):

pluggable = can be swapped out

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:50):

No, what I'm afraid of is that we put it somewhere outside of rustdoc

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:50):

aaaah

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:50):

@GuillaumeGomez can you expand on what you mean here?

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:50):

if that's what you mean, I intended to disable it by default at first

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:50):

my concern is _maintenance_ wise

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:50):

it should perhaps be outside of rustdoc

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:51):

OR it should be well-scoped enough that the rustdoc team is happy with taking on the maintenance efforts

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:51):

It's tightly bound to rustdoc and shouldn't be put outside of rustdoc

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:51):

and I'm happy with taking the maintenance for it (I'm paid to work on rustdoc now so I have more time now)

view this post on Zulip CraftSpider (Apr 07 2021 at 15:51):

Intra-doc links, no. They're more fundamental, and I believe much harder to implement that way. Themes already are customizable, just in a different way.
Implementing it this way gets two advantages:
1) If we want it ourselves, this way doesn't stop that, but it also allows us to not
2) I think a good design would often look like this to some degree anyways, this ensures forward compat is considered.

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:52):

@GuillaumeGomez I don't think a single person being able to maintain a thing is sufficient

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:52):

@CraftSpider Didn't understand your explanations at all, sorry :s

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:52):

@GuillaumeGomez also stepping back a bit for _good_ design i actually think the pluggable thing is better -- I personally have _often_ wanted to generate docs that link to github

view this post on Zulip CraftSpider (Apr 07 2021 at 15:52):

Ah, sorry. Give me a minute, I'll be back not on my phone

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:52):

so having --srclinks plain --srclinks github --srclinks fancy

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:53):

idk

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:53):

people with size constraints can do --srclinks plain

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:53):

I don't like this approach but I think it's a point of view thing here

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:53):

or even --srclinks none

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:53):

like i think there's a lot of cool stuff we could do if there's a flag like we have for themes

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:53):

Manish Goregaokar said:

GuillaumeGomez also stepping back a bit for _good_ design i actually think the pluggable thing is better -- I personally have _often_ wanted to generate docs that link to github

docs.rs also wants to do this

view this post on Zulip Joshua Nelson (Apr 07 2021 at 15:54):

this is exactly why I think there should be an RFC, so stakeholders have a place to say what would be useful

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:54):

what I have in mind is --enable-src-definitions until we have a good enough thing (or an RFC)

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:54):

sure

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:54):

But I agree that it should disabled

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:54):

happy to land a proof of concept as long as it is small enough

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:55):

if that's what a plugin means to you, we agree on that

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:55):

well

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 15:55):

i hold multiple opinions here

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:55):

What i want to avoid is having it in a different repository

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 15:55):

(or in a different crate)

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 16:04):

Firstly, I think we should be very clear on the scope of this. An RFC is enough for that. We should ask in a pre-rfc post what people would like to see in such a feature.

If the scope is pretty large, I would prefer it to be a different "project" (can just be a folder under src/tools/rustdoc or src/tools), with perhaps its own list of maintainers (OWNERS file if it's under rustdoc/), and perhaps able to be run on its own (i would _love_ that). rustdoc can integrate directly into it so from the user's pov it doesn't matter. This is _entirely_ a maintenance concern. The user doesn't need to care.

by pluggable I mean that for src generation we could have a set of traits and you can write different src generators. Because these APIs are unstable basically only rustdoc will write such src generators, but my main concern is that I don't want src generation stuff to leak into the rest of the code.

So then rustdoc could have html/srcgen/plain and srcgen/fancy and srcgen/github folders, and each would implement this trait. that way it's _clean_, we don't have to worry about it when working on the rest of rustdoc

I do feel like if we did an RFC about it folks will bring up the github thing and that's why I'm focusing on pluggability; give rustdoc a single API for this, and we write multiple srcgen thingies for it

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 16:07):

(unsure if this is what @Joshua Nelson means)

view this post on Zulip Joshua Nelson (Apr 07 2021 at 16:07):

basically yes

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 16:07):

but that's mostly what i'm looking for @GuillaumeGomez

view this post on Zulip CraftSpider (Apr 07 2021 at 16:07):

Okay, Manish basically just summed up what I was going to say. (Sorry for the delay, Windows Update ate my laptop for a bit)

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 16:07):

i think having a trait API for this would make it much much easier to maintain

view this post on Zulip Joshua Nelson (Apr 07 2021 at 16:08):

if there's an explicit opt-in for github then docs.rs can say "use github for things with a github repo and use the built-in src viewer otherwise"

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 16:08):

intra doc links is scattered through the code and it's not great but it's also ... a one time thing

view this post on Zulip Joshua Nelson (Apr 07 2021 at 16:08):

or even use its own source viewer instead of rustdoc's

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 16:08):

--srclinks=github=manishearth/elsa:1.2.3

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 16:08):

idk

view this post on Zulip Joshua Nelson (Apr 07 2021 at 16:09):

oh I guess we do have to pass it for each crate at once, since you can view the source of an item re-exported from another crate

view this post on Zulip Joshua Nelson (Apr 07 2021 at 16:09):

see this is exactly why an RFC would be useful :laughing:

view this post on Zulip CraftSpider (Apr 07 2021 at 16:09):

The specifics can be an RFC, but I like the overall idea proposed :P

view this post on Zulip CraftSpider (Apr 07 2021 at 16:11):

To note that one reason I approve of this kind of design is the reason JSON bugs have some pain points: HTML was the only generator for so long, it's woven forward and back through the system. There isn't as clear a line of 'backend/generator', meaning that even entirely within rustdoc JSON leaks HTML implementation details sometimes.
Adopting a separated design from the start saves you from later issues, meaning we don't need external tools, but we make sure we aren't accidentally shooting ourselves in the foot down the line

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 16:15):

Adopting a separated design from the start saves you from later issues, meaning we don't need external tools, but we make sure we aren't accidentally shooting ourselves in the foot down the line

Yeah this is my core point

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 16:15):

well i have many core points

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 16:15):

but i want to +1 that

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 17:14):

I think this is over-engineering. Way too much. We're talking about a feature in the HTML generator. Just like no one (I know of) is using their own theme, I don't see anyone adding their own source code generator. Making things generic and extendable is rarely a good idea if it's not meant to be. But anyway, it's still far into the future, for now I'm still working on the POC that will remain disabled by default until we reach an agreement on what this feature should look like. Do you think it's fine as a start?

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 18:58):

@GuillaumeGomez No, I'm not saying we should do this to enable others to write their own srcgen (I explicitly said earlier: it's going to be an unstable API, nobody will want to use it), I'm saying we should do this to make it easy to maintain internally

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 18:58):

because my _guess_ is that it's goign to end up being way more complex than your current PR; and I want that code abstracted by a trait

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 18:59):

and _if we do that_ it becomes easy to add "plain" srcdoc and "link to github" srcdoc

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 18:59):

the latter is definitely something people want as well, and i'm talking about it because a unified solution is nice

view this post on Zulip GuillaumeGomez (Apr 07 2021 at 19:04):

Ah I see, makes sense.

view this post on Zulip Manish Goregaokar (Apr 07 2021 at 19:06):

like, i have two primary concerns here:

the former is helped by abstracting it away properly (and as a bonus we can make --srcdoc github work later!). the latter is helped by an rfc and an internals post so we know how far we have to go with this


Last updated: Oct 11 2021 at 22:34 UTC