Repository: rust-analyzer/rust-analyzer
Pull request #2715 (2019-12-31 20:13:45)

Fixes to progress display


Patch:
diff --git a/crates/ra_mbe/src/mbe_expander/transcriber.rs b/crates/ra_mbe/src/mbe_expander/transcriber.rs
index 7662020f36..eda66cd506 100644
--- a/crates/ra_mbe/src/mbe_expander/transcriber.rs
+++ b/crates/ra_mbe/src/mbe_expander/transcriber.rs
@@ -14,24 +14,21 @@ impl Bindings {
         self.inner.contains_key(name)
     }
 
-    fn get(&self, name: &str, nesting: &mut [NestingState]) -> Result<&Fragment, ExpandError> {
+    fn get(&self, name: &str, nesting: &[usize]) -> Result<&Fragment, ExpandError> {
         let mut b = self.inner.get(name).ok_or_else(|| {
             ExpandError::BindingError(format!("could not find binding `{}`", name))
         })?;
-        for nesting_state in nesting.iter_mut() {
-            nesting_state.hit = true;
+        for &idx in nesting.iter() {
             b = match b {
                 Binding::Fragment(_) => break,
-                Binding::Nested(bs) => bs.get(nesting_state.idx).ok_or_else(|| {
-                    nesting_state.at_end = true;
+                Binding::Nested(bs) => bs.get(idx).ok_or_else(|| {
                     ExpandError::BindingError(format!("could not find nested binding `{}`", name))
                 })?,
                 Binding::Empty => {
-                    nesting_state.at_end = true;
                     return Err(ExpandError::BindingError(format!(
                         "could not find empty binding `{}`",
                         name
-                    )));
+                    )))
                 }
             };
         }
@@ -54,25 +51,15 @@ pub(super) fn transcribe(
     bindings: &Bindings,
 ) -> Result<tt::Subtree, ExpandError> {
     assert!(template.delimiter == None);
-    let mut ctx = ExpandCtx { bindings: &bindings, nesting: Vec::new() };
+    let mut ctx = ExpandCtx { bindings: &bindings, nesting: Vec::new(), var_expanded: false };
     expand_subtree(&mut ctx, template)
 }
 
-#[derive(Debug)]
-struct NestingState {
-    idx: usize,
-    /// `hit` is currently necessary to tell `expand_repeat` if it should stop
-    /// because there is no variable in use by the current repetition
-    hit: bool,
-    /// `at_end` is currently necessary to tell `expand_repeat` if it should stop
-    /// because there is no more value avaible for the current repetition
-    at_end: bool,
-}
-
 #[derive(Debug)]
 struct ExpandCtx<'a> {
     bindings: &'a Bindings,
-    nesting: Vec<NestingState>,
+    nesting: Vec<usize>,
+    var_expanded: bool,
 }
 
 fn expand_subtree(ctx: &mut ExpandCtx, template: &tt::Subtree) -> Result<tt::Subtree, ExpandError> {
@@ -134,7 +121,9 @@ fn expand_var(ctx: &mut ExpandCtx, v: &SmolStr) -> Result<Fragment, ExpandError>
         .into();
         Fragment::Tokens(tt)
     } else {
-        ctx.bindings.get(&v, &mut ctx.nesting)?.clone()
+        let fragment = ctx.bindings.get(&v, &ctx.nesting)?.clone();
+        ctx.var_expanded = true;
+        fragment
     };
     Ok(res)
 }
@@ -146,24 +135,37 @@ fn expand_repeat(
     separator: Option<Separator>,
 ) -> Result<Fragment, ExpandError> {
     let mut buf: Vec<tt::TokenTree> = Vec::new();
-    ctx.nesting.push(NestingState { idx: 0, at_end: false, hit: false });
+    ctx.nesting.push(0);
     // Dirty hack to make macro-expansion terminate.
     // This should be replaced by a propper macro-by-example implementation
-    let limit = 65536;
+    let mut limit = 65536;
     let mut has_seps = 0;
     let mut counter = 0;
 
-    loop {
-        let res = expand_subtree(ctx, template);
-        let nesting_state = ctx.nesting.last_mut().unwrap();
-        if nesting_state.at_end || !nesting_state.hit {
+    // We store the old var expanded value, and restore it later
+    // It is because before this `$repeat`,
+    // it is possible some variables already expanad in the same subtree
+    //
+    // `some_var_expanded` keep check if the deeper subtree has expanded variables
+    let mut some_var_expanded = false;
+    let old_var_expanded = ctx.var_expanded;
+    ctx.var_expanded = false;
+
+    while let Ok(mut t) = expand_subtree(ctx, template) {
+        t.delimiter = None;
+        // if no var expanded in the child, we count it as a fail
+        if !ctx.var_expanded {
             break;
         }
-        nesting_state.idx += 1;
-        nesting_state.hit = false;
+
+        // Reset `ctx.var_expandeded` to see if there is other expanded variable
+        // in the next matching
+        some_var_expanded = true;
+        ctx.var_expanded = false;
 
         counter += 1;
-        if counter == limit {
+        limit -= 1;
+        if limit == 0 {
             log::warn!(
                 "expand_tt excced in repeat pattern exceed limit => {:#?}\n{:#?}",
                 template,
@@ -172,11 +174,8 @@ fn expand_repeat(
             break;
         }
 
-        let mut t = match res {
-            Ok(t) => t,
-            Err(_) => continue,
-        };
-        t.delimiter = None;
+        let idx = ctx.nesting.pop().unwrap();
+        ctx.nesting.push(idx + 1);
         push_subtree(&mut buf, t);
 
         if let Some(ref sep) = separator {
@@ -204,6 +203,9 @@ fn expand_repeat(
         }
     }
 
+    // Restore the `var_expanded` by combining old one and the new one
+    ctx.var_expanded = some_var_expanded || old_var_expanded;
+
     ctx.nesting.pop().unwrap();
     for _ in 0..has_seps {
         buf.pop();
diff --git a/editors/code/src/main.ts b/editors/code/src/main.ts
index 51dedd5efc..430ad31b4c 100644
--- a/editors/code/src/main.ts
+++ b/editors/code/src/main.ts
@@ -11,17 +11,6 @@ let ctx!: Ctx;
 export async function activate(context: vscode.ExtensionContext) {
     ctx = new Ctx(context);
 
-    // Note: we try to start the server before we register various commands, so
-    // that it registers its `onDidChangeDocument` handler before us.
-    //
-    // This a horribly, horribly wrong way to deal with this problem.
-    try {
-        await ctx.restartServer();
-    } catch (e) {
-        vscode.window.showErrorMessage(e.message);
-    }
-
-
     // Commands which invokes manually via command pallet, shortcut, etc.
     ctx.registerCommand('analyzerStatus', commands.analyzerStatus);
     ctx.registerCommand('collectGarbage', commands.collectGarbage);
@@ -42,7 +31,17 @@ export async function activate(context: vscode.ExtensionContext) {
         ctx.overrideCommand('type', commands.onEnter);
     }
     activateStatusDisplay(ctx);
+
     activateHighlighting(ctx);
+    // Note: we try to start the server before we activate type hints so that it
+    // registers its `onDidChangeDocument` handler before us.
+    //
+    // This a horribly, horribly wrong way to deal with this problem.
+    try {
+        await ctx.restartServer();
+    } catch (e) {
+        vscode.window.showErrorMessage(e.message);
+    }
     activateInlayHints(ctx);
 }
 
diff --git a/editors/code/src/status_display.ts b/editors/code/src/status_display.ts
index 08cdc8bdf6..371a2f3bb0 100644
--- a/editors/code/src/status_display.ts
+++ b/editors/code/src/status_display.ts
@@ -36,13 +36,9 @@ class StatusDisplay implements vscode.Disposable {
             this.timer ||
             setInterval(() => {
                 if (this.packageName) {
-                    this.statusBarItem!.text = `cargo ${this.command} [${
-                        this.packageName
-                        }] ${this.frame()}`;
+                    this.statusBarItem!.text = `${this.frame()} cargo ${this.command} [${this.packageName}]`;
                 } else {
-                    this.statusBarItem!.text = `cargo ${
-                        this.command
-                        } ${this.frame()}`;
+                    this.statusBarItem!.text = `${this.frame()} cargo ${this.command}`;
                 }
             }, 300);
 


Test patch:
diff --git a/crates/ra_mbe/src/tests.rs b/crates/ra_mbe/src/tests.rs
index e0d689704b..e640d115b4 100644
--- a/crates/ra_mbe/src/tests.rs
+++ b/crates/ra_mbe/src/tests.rs
@@ -1491,51 +1491,3 @@ fn debug_dump_ignore_spaces(node: &ra_syntax::SyntaxNode) -> String {
 
     buf
 }
-
-#[test]
-fn test_issue_2520() {
-    let macro_fixture = parse_macro(
-        r#"
-        macro_rules! my_macro {
-            {
-                ( $(
-                    $( [] $sname:ident : $stype:ty  )?
-                    $( [$expr:expr] $nname:ident : $ntype:ty  )?
-                ),* )
-            } => {
-                Test {
-                    $(
-                        $( $sname, )?
-                    )*
-                }
-            };
-        }
-    "#,
-    );
-
-    macro_fixture.assert_expand_items(
-        r#"my_macro ! {
-            ([] p1 : u32 , [|_| S0K0] s : S0K0 , [] k0 : i32)
-        }"#,
-        "Test {p1 , k0 ,}",
-    );
-}
-
-#[test]
-fn test_repeat_bad_var() {
-    // FIXME: the second rule of the macro should be removed and an error about
-    // `$( $c )+` raised
-    parse_macro(
-        r#"
-        macro_rules! foo {
-            ($( $b:ident )+) => {
-                $( $c )+
-            };
-            ($( $b:ident )+) => {
-                $( $b )+
-            }
-        }
-    "#,
-    )
-    .assert_expand_items("foo!(b0 b1);", "b0 b1");
-}