-
Notifications
You must be signed in to change notification settings - Fork 931
Format loops #128
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
Format loops #128
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,11 +12,13 @@ use rewrite::{Rewrite, RewriteContext}; | |
use lists::{write_list, itemize_list, ListFormatting, SeparatorTactic, ListTactic}; | ||
use string::{StringFormat, rewrite_string}; | ||
use utils::{span_after, make_indent}; | ||
use visitor::FmtVisitor; | ||
|
||
use syntax::{ast, ptr}; | ||
use syntax::codemap::{Pos, Span, BytePos}; | ||
use syntax::codemap::{Pos, Span, BytePos, mk_sp}; | ||
use syntax::parse::token; | ||
use syntax::print::pprust; | ||
use syntax::visit::Visitor; | ||
|
||
impl Rewrite for ast::Expr { | ||
fn rewrite(&self, context: &RewriteContext, width: usize, offset: usize) -> Option<String> { | ||
|
@@ -53,11 +55,38 @@ impl Rewrite for ast::Expr { | |
ast::Expr_::ExprTup(ref items) => { | ||
rewrite_tuple_lit(context, items, self.span, width, offset) | ||
} | ||
ast::Expr_::ExprLoop(ref block, _) => { | ||
// FIXME: this drops any comment between "loop" and the block. | ||
// TODO: format label | ||
block.rewrite(context, width, offset).map(|result| { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. DO you need to adjust the width here to take account of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think so. Blocks go to the new line immediately after the opening brace.. That's why There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes, of course |
||
format!("loop {}", result) | ||
}) | ||
} | ||
_ => context.codemap.span_to_snippet(self.span).ok() | ||
} | ||
} | ||
} | ||
|
||
impl Rewrite for ast::Block { | ||
fn rewrite(&self, context: &RewriteContext, _: usize, _: usize) -> Option<String> { | ||
let mut visitor = FmtVisitor::from_codemap(context.codemap, context.config); | ||
visitor.last_pos = self.span.lo; | ||
visitor.block_indent = context.block_indent; | ||
|
||
visitor.visit_block(self); | ||
|
||
// Push text between last block item and end of block | ||
let snippet = visitor.snippet(mk_sp(visitor.last_pos, self.span.hi)); | ||
visitor.changes.push_str_span(self.span, &snippet); | ||
|
||
// Stringify visitor | ||
let file_name = context.codemap.span_to_filename(self.span); | ||
let string_buffer = visitor.changes.get(&file_name); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not saying you need to make the changes as part of this PR, but just going to note some odd-ness about the FmtVisitor API here:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I thought a bit about that when dealing with molules issue. In fact, there are only two places where we need the multiple-file aspect of the visitor: visiting a module and when we print the result. For all other tasks, complicated changeset is only noise. For example, there is no obvious reason for |
||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed; fixed. |
||
Some(string_buffer.to_string()) | ||
} | ||
} | ||
|
||
fn rewrite_string_lit(context: &RewriteContext, | ||
s: &str, | ||
span: Span, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
|
||
fn main() { | ||
loop | ||
{ return some_val;} | ||
|
||
let x = loop { do_forever(); }; | ||
|
||
loop { | ||
// Just comments | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
|
||
fn main() { | ||
loop { | ||
return some_val; | ||
} | ||
|
||
let x = loop { | ||
do_forever(); | ||
}; | ||
|
||
loop { | ||
// Just comments | ||
} | ||
} |
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.
Should document that this panics if file_name is unknown (also for get_mut)