-
Notifications
You must be signed in to change notification settings - Fork 12.9k
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
rustdoc: print non-self arguments of bare functions and struct methods on their own line #36679
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @steveklabnik (or someone else) soon. If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes. Please see the contribution instructions for more information. |
Yeah, with the shorter signatures the added newlines can be a net negative as far as noise is concerned. On the other hand, I'd prefer it to be consistent with longer signatures, rather than saying "minimum one non-self argument before adding linebreaks" or something like that. |
The arguments should at least be aligned with the opening parenthesis if they have to go on the new line. It seems like a length threshold would be good. |
This is the second PR about code formatting in rustdoc formatting in an hour. Plus there's #36654 which isn't even a day old. Where's all the rustdoc code formatting buzz coming from all of a sudden? 😄 Don't get me wrong, I'm not complaining. |
As for the specific change here, I agree that it's excessive to do on virtually all signatures. Perhaps running rustdoc (like #36654 proposes for type definitions) on the signatures will be better? It would keep short signatures (regardless of the exact number of parameters) on one line but still split bigger signatures. |
@durka I like that suggestion for the indentation. It'll be weird getting the indent level passed in due to how it's being printed, but I think it's doable. As for a length threshold, I'll echo my earlier comment about consistency. Adding newlines based on the final length of the argument string feels like a weird thing to base the decision on, especially if short type parameters get involved. It feels like it would be worse to have three arguments fit in one line simply because they have a short binding name and type name; it would clutter up the line the same way longer signatures would now. |
@QuietMisdreavus But thresholding based on the length is already done in code! Both rustfmt and most people's personal style will break signatures based on length, not on argument count. |
@rkruppe Except this is documentation, not code. It's not read the same way, and it doesn't need to be formatted the same way. IMO it's okay to have extra vertical space in docs because it makes the interface of the library more explicit, and aids first-time reading comprehension to see all the arguments laid out. In the actual code, you can format it differently because by the time you look at that version you're investigating how it works or already have that understanding. I ran rustfmt (via the playground) on a sample of code from egg-mode (and a handful of other function definitions outside this gist) because I personally hadn't seen its output on these kinds of cases before. It seems to agree with the "one argument per line" rule, but only once the signature passes some length threshold. That's easy enough to set up, provided I can get the length of the leading name into the Display impl. I surveyed a handful of other languages' standard documentations to see what styles were in use. It's hard to get a real apples-to-apples comparison, but at least I can get some concrete examples to reference for good or ill.
I think it's the last approach that I'm against; arguments might have a logical grouping that would get broken up by the "fit as many on a line as you can" style. At the level that rustdoc is working, I see no way to copy any potentially-existing manual grouping from the original source code, either. I'd be okay with a sort of "all or nothing" style, where signatures can fit on one line under some threshold (which would be calculated by... byte count? |
This is a good idea but its exploitation isn't good from my point of view. I don't like backline forcing when it's unneeded. Also:
This is right and wrong at the same time. If its format is close to the code, people will read it more quickly. However, since it's doc, we can improve it more than the code could allow it. |
@rkruppe Why now? Because we have docs.rs now. So instead of applying my own css tweaks and host docs myself I have to try to push upstream. |
@rkruppe @bluss For me, the timing is purely coincidence. I mused on IRC about wanting to have this change, then I researched how to make it happen. I figure, even if this is ultimately rejected and I wind up keeping my own rustdoc fork for my personally-hosted docs, the process of trying to get it accepted upstream will most likely make it better than my initial naive implementation. |
@GuillaumeGomez I guess what I'm trying to distinguish is that reading code to understand how it works and how to change it is different from reading code (and accompanying text) to understand how to use it from an outside/black-box perspective. What works in the compiled code to allow more seamless access to neighboring methods or to group related arguments together may not work as well in the separate documentation when I'm trying to figure out what parameter goes where and which one I'm missing. I cut my teeth in .NET where I had ample documentation and didn't need to see the inner workings as much. (It's also the documentation-code-block indentation style I was aping in the provided initial samples.) I'll recognize that that may not work as well here, but it's at least the direction I'm coming from. |
@QuietMisdreavus: Like I said, I like your idea, but not when a function has only one parameter. Maybe we should think about it when we have more than one parameter? Hard to say. |
My feeling is that if we do this, rustdoc should follow rustfmt (and thus the official formatting guidelines, as they are decided) strictly. Preferably by actually calling Rustfmt (in process, it has API that should be able to handle this). It seems very bad to use different formatting here vs in Rust code. In particular, we should probably only do this for 'long' signatures, I think for signatures that are short, this is not an improvement (but it is for longer sigs). |
Calling rustfmt directly would be difficult, since the same process that prints this information also writes the HTML anchor tags inline. I'm in the middle of wrangling the indentation rules so that it will match rustfmt's guidelines. Just so that it can get said (or pointed towards) directly, what should we qualify as a "long" signature? What character length (if that's our standard) is "long" here? |
Rustfmt breaks at the max line length, which is currently 100, but there is some debate about lowering it to 80 (and this is used in much of std, but not the compiler). Given the larger font here, I would use at most 80, and maybe less, depending on where we start wrapping on a typical display. |
* ignore signatures 80 characters or shorter * otherwise, put arguments and return arrow on their own line, indented to the opening parenthesis in doing this, most of a plain-text output has been written for rustdoc, accessible through "alternate" Display printing: "{:#}"
see commit 2a274e7 for details
I've made some updates, to make the function formatting more like rustfmt. The rules are implemented as follows:
Implementing this amounted to creating a plain-text output for functions and struct methods, so that I wouldn't have to parse out the link tags and html entities. The code could use some style pointers. The examples linked above should look much nicer now. There are still weird edge cases with awkward signatures, but it should be nicer than my earlier appropriation of .NET documentation style. |
I appreciate this a bit more. I wait to see how it's going to evolve to give my approval but I think it's on the good way! |
Please do the breaking in the browser, so if someone views the docs on a phone the arguments will be on new lines earlier than when shown on a pc. |
@nrc what do you think of this now? |
LGTM. Long-term I'd love to find a way to use Rustfmt directly, but this definitely looks like an improvement. |
Let's Thank you for the patience, @QuietMisdreavus . @bors: r+ rollup |
📌 Commit 0c2b258 has been approved by |
…, r=steveklabnik rustdoc: print non-self arguments of bare functions and struct methods on their own line This change alters the formatting rustdoc uses when it creates function and struct method documentation. For bare functions, each argument is printed on its own line. For struct methods, non-self arguments are printed on their own line. In both cases, no line breaks are introduced if there are no arguments, and for struct methods, no line breaks are introduced if there is only a single self argument. This should aid readability of long function signatures and allow for greater comprehension of these functions. I've run rustdoc with these changes on my crate egg-mode and its set of dependencies and put the result [on my server](https://shiva.icesoldier.me/doc-custom/egg_mode/). Of note, here are a few shortcut links that highlight the changes: * [Bare function with a long signature](https://shiva.icesoldier.me/doc-custom/egg_mode/place/fn.reverse_geocode.html) * [Struct methods, with single self argument and with self and non-self arguments](https://shiva.icesoldier.me/doc-custom/egg_mode/tweet/struct.Timeline.html#method.reset) * [Bare functions with no arguments](https://shiva.icesoldier.me/doc-custom/rand/fn.thread_rng.html) and [struct methods with no arguments](https://shiva.icesoldier.me/doc-custom/hyper/client/struct.Client.html#method.new) are left unchanged. This PR consists of two commits: one for bare functions and one for struct methods.
Nice. Have you considered line breaking long where clauses? 😄 Example function in rustdoc |
…ne, r=GuillaumeGomez rustdoc: add line breaks to where clauses a la rustfmt Much like my last PR for rustdoc (rust-lang#36679), this adds line breaks to certain statements based on their line length. Here the focus was on where clauses. Some examples: - [Where clause in a trait function](https://shiva.icesoldier.me/custom-std/std/iter/trait.Iterator.html?search=#method.unzip) (also in the trait header block at the top of the page) - [Where clause on a bare function](https://shiva.icesoldier.me/doc-custom2/petgraph/visit/fn.depth_first_search.html) - [Where clauses in trait impls on a struct](https://shiva.icesoldier.me/custom-std/std/collections/struct.HashMap.html) (scroll to the bottom) These are regularly not on their own line, but will be given their own line now if their "prefix text" doesn't give them enough room to sensibly print their constraints. HashMap's trait impls provide some examples of both behaviors. The libstd links above are the whole docs rendered with this, and the "bare function" link above is in another set that pulls some notable crates together. `petgraph` was the one that brought this request up, and that collection also includes [itertools](https://shiva.icesoldier.me/doc-custom2/itertools/trait.Itertools.html) which provided an easy sample to test with. r? @GuillaumeGomez
…ne, r=GuillaumeGomez rustdoc: add line breaks to where clauses a la rustfmt Much like my last PR for rustdoc (rust-lang#36679), this adds line breaks to certain statements based on their line length. Here the focus was on where clauses. Some examples: - [Where clause in a trait function](https://shiva.icesoldier.me/custom-std/std/iter/trait.Iterator.html?search=#method.unzip) (also in the trait header block at the top of the page) - [Where clause on a bare function](https://shiva.icesoldier.me/doc-custom2/petgraph/visit/fn.depth_first_search.html) - [Where clauses in trait impls on a struct](https://shiva.icesoldier.me/custom-std/std/collections/struct.HashMap.html) (scroll to the bottom) These are regularly not on their own line, but will be given their own line now if their "prefix text" doesn't give them enough room to sensibly print their constraints. HashMap's trait impls provide some examples of both behaviors. The libstd links above are the whole docs rendered with this, and the "bare function" link above is in another set that pulls some notable crates together. `petgraph` was the one that brought this request up, and that collection also includes [itertools](https://shiva.icesoldier.me/doc-custom2/itertools/trait.Itertools.html) which provided an easy sample to test with. r? @GuillaumeGomez
This change alters the formatting rustdoc uses when it creates function and struct method documentation. For bare functions, each argument is printed on its own line. For struct methods, non-self arguments are printed on their own line. In both cases, no line breaks are introduced if there are no arguments, and for struct methods, no line breaks are introduced if there is only a single self argument. This should aid readability of long function signatures and allow for greater comprehension of these functions.
I've run rustdoc with these changes on my crate egg-mode and its set of dependencies and put the result on my server. Of note, here are a few shortcut links that highlight the changes:
This PR consists of two commits: one for bare functions and one for struct methods.