Minor librustdoc cleanup and refactoring. #36903

frewsxcv · 2016-10-02T01:22:22Z

No description provided.

rust-highfive · 2016-10-02T01:22:29Z

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.

frewsxcv · 2016-10-02T01:22:29Z

r? @GuillaumeGomez

frewsxcv · 2016-10-02T01:25:46Z

I need a sanity check for 19f6130

Here's the new print function it will use:

rust/src/librustc/hir/print.rs

Lines 273 to 275 in df9fa1a

    
           pub fn path_to_string(p: &hir::Path) -> String { 
        
               to_string(|s| s.print_path(p, false, 0)) 
        
           }

rust/src/librustc/hir/print.rs

Lines 1615 to 1636 in df9fa1a

    
           fn print_path(&mut self, 
        
                         path: &hir::Path, 
        
                         colons_before_params: bool, 
        
                         depth: usize) 
        
                         -> io::Result<()> { 
        
               self.maybe_print_comment(path.span.lo)?; 
        
               let mut first = !path.global; 
        
               for segment in &path.segments[..path.segments.len() - depth] { 
        
                   if first { 
        
                       first = false 
        
                   } else { 
        
                       word(&mut self.s, "::")? 
        
                   } 
        
                   self.print_name(segment.name)?; 
        
                   self.print_path_parameters(&segment.parameters, colons_before_params)?; 
        
               } 
        
               Ok(()) 
        
           }

jseyfried

r=me modulo comments.

jseyfried · 2016-10-02T20:59:30Z

src/librustdoc/clean/mod.rs

-        CLikeVariant
-    } else {
-        TupleVariant(struct_def.fields().iter().map(|x| x.ty.clean(cx)).collect())
+impl VariantKind {


Could you make this an implementation of Clean<VariantKind> for hir::VariantData?

d6fb0c8d6a01beab132f1df7832794401ac92a46

jseyfried · 2016-10-02T21:18:53Z

src/librustdoc/clean/mod.rs

@@ -2682,11 +2668,12 @@ fn name_from_pat(p: &hir::Pat) -> String {
    match p.node {
        PatKind::Wild => "_".to_string(),
        PatKind::Binding(_, ref p, _) => p.node.to_string(),
-        PatKind::TupleStruct(ref p, ..) | PatKind::Path(None, ref p) => path_to_string(p),
+        PatKind::TupleStruct(ref p, ..) | PatKind::Path(None, ref p) => p.to_string(),


hir::print::State::print_path begins with self.maybe_print_comment(path.span.lo), which might actually print a comment in this case:

fn f(arg1: (), // Some comment path::to::TupleStruct(x): path::to::TupleStruct) { //^ The `name_from_pat` of this tuple struct pattern might include "// Some comment" }

Do you think I should add an parameter to that function to control whether the comment gets printed or just ditch that commit?

I'd probably ditch the commit to be on the safe side since I don't understand the pretty printer, especially in edge cases (e.g. it might emit newlines and indents in long patterns).

jseyfried · 2016-10-03T01:14:07Z

src/librustdoc/clean/mod.rs

+            VariantKind::CLike
+        } else {
+            VariantKind::Tuple(
+                self.fields().iter().map(|x| x.ty.clean(cx)).collect())


nit: this can fit on one line

jseyfried · 2016-10-03T01:14:18Z

src/librustdoc/clean/mod.rs

-                (ret, ImportList(resolve_use_source(cx, p.clean(cx), self.id),
-                                 remaining))
+                (ret, Import::List(resolve_use_source(cx, p.clean(cx), self.id),
+                                   remaining))


nit: this can fit on one line

jseyfried · 2016-10-03T01:20:33Z

src/librustdoc/html/render.rs

+            use clean::{Variant, VariantKind};
+            if let clean::VariantItem(
+                Variant { kind: VariantKind::Struct(ref s) } ) = variant.inner
+            {


nit: this would be easier for me to read:

if let clean::VariantItem(Variant { kind: VariantKind::Struct(ref s) }) = variant.inner {

or

if let clean::VariantItem(Variant { kind: VariantKind::Struct(ref s) }) = variant.inner {

rust-lang#36903 (comment)

frewsxcv · 2016-10-03T01:58:29Z

Latest force push removes 19f6130 and makes the suggested style changes.

jseyfried · 2016-10-03T02:03:09Z

Thanks! @bors r+

bors · 2016-10-03T02:03:10Z

📌 Commit 35d214a has been approved by jseyfried

Minor librustdoc cleanup and refactoring.

Rollup of 12 pull requests - Successful merges: #36798, #36878, #36902, #36903, #36908, #36916, #36917, #36921, #36928, #36938, #36941, #36951 - Failed merges:

rust-lang/rust#36903 (comment)

rust-highfive assigned steveklabnik Oct 2, 2016

rust-highfive assigned GuillaumeGomez and unassigned steveklabnik Oct 2, 2016

jseyfried reviewed Oct 2, 2016

View reviewed changes

jseyfried reviewed Oct 3, 2016

View reviewed changes

frewsxcv added 8 commits October 2, 2016 21:58

Remove redundant 'Type' in variant names, stop reexporting.

c3bc905

Rename method 'to_string' to match conventions.

0d0f1b4

Migrate VariantKind construction function to associated function.

6d08949

Cleanup return statements.

a400ccc

Remove redundant 'Import' in variant names, stop reexporting.

5b9ba4c

Simplify equality checks.

b55468c

Migrate VariantKind constructor to Clean impl.

88d4144

rust-lang#36903 (comment)

Remove redundant 'Variant' in variant names, stop reexporting.

35d214a

frewsxcv force-pushed the typekind branch from 44d86fc to 35d214a Compare October 3, 2016 01:58

Manishearth mentioned this pull request Oct 4, 2016

Rollup of 12 pull requests #36953

Merged

Manishearth added a commit to Manishearth/rust that referenced this pull request Oct 4, 2016

Rollup merge of rust-lang#36903 - frewsxcv:typekind, r=jseyfried

825e6b3

Minor librustdoc cleanup and refactoring.

bors added a commit that referenced this pull request Oct 4, 2016

Auto merge of #36953 - Manishearth:rollup, r=Manishearth

4a9af01

Rollup of 12 pull requests - Successful merges: #36798, #36878, #36902, #36903, #36908, #36916, #36917, #36921, #36928, #36938, #36941, #36951 - Failed merges:

bors merged commit 35d214a into rust-lang:master Oct 4, 2016

critiqjo pushed a commit to critiqjo/rustdoc that referenced this pull request Dec 16, 2016

Migrate VariantKind constructor to Clean impl.

a35cd41

rust-lang/rust#36903 (comment)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Minor librustdoc cleanup and refactoring. #36903

Minor librustdoc cleanup and refactoring. #36903

frewsxcv commented Oct 2, 2016

rust-highfive commented Oct 2, 2016

frewsxcv commented Oct 2, 2016

frewsxcv commented Oct 2, 2016

jseyfried left a comment

jseyfried Oct 2, 2016

frewsxcv Oct 3, 2016

jseyfried Oct 2, 2016 •

edited

Loading

frewsxcv Oct 3, 2016

jseyfried Oct 3, 2016

jseyfried Oct 3, 2016

jseyfried Oct 3, 2016

jseyfried Oct 3, 2016

jseyfried Oct 3, 2016

frewsxcv commented Oct 3, 2016

jseyfried commented Oct 3, 2016

bors commented Oct 3, 2016

Minor librustdoc cleanup and refactoring. #36903

Minor librustdoc cleanup and refactoring. #36903

Conversation

frewsxcv commented Oct 2, 2016

rust-highfive commented Oct 2, 2016

frewsxcv commented Oct 2, 2016

frewsxcv commented Oct 2, 2016

jseyfried left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jseyfried Oct 2, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

frewsxcv commented Oct 3, 2016

jseyfried commented Oct 3, 2016

bors commented Oct 3, 2016

jseyfried Oct 2, 2016 •

edited

Loading