Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

Filter out variables on Outline view #58

Closed
aldonogueira opened this issue Feb 21, 2018 · 8 comments · Fixed by #65
Closed

Filter out variables on Outline view #58

aldonogueira opened this issue Feb 21, 2018 · 8 comments · Fixed by #65

Comments

@aldonogueira
Copy link
Contributor

The outline view would be more useful if there were a way to filter out the variables. It seems most of the times developers are interested in functions, traits and structs, but the variables make them sparse.

I don't know the proper way to implement this behaviour though.

@aldonogueira aldonogueira changed the title Being able to filter out variables on Outline view Filter out variables on Outline view Feb 21, 2018
@alexheretic
Copy link
Member

Currently ide-rust totally relies on upstream atom-languageclient & atom-ide-ui for outlines. I think, in general, adding new functionality upstream and then configuring it best for rust here makes sense.

At the same time if you want to raise a PR to add a filter option for variables in the outline, I'd probably accept it as we can always drop the config when that functionality appears upstream.

@aldonogueira
Copy link
Contributor Author

The interaction between components are way more complex than I thought. I'm not sure which piece of software should be modified to properly show hierarchy of items in the outline. I'll take a look at it again over the weekend

@aldonogueira
Copy link
Contributor Author

It looks there is already an issue to remove local variables from the RLS. I guess they may be useful in other languages, but in Rust's outline they seem unnecessary

@aldonogueira
Copy link
Contributor Author

I made a PR in RLS to remove local variables, but people thought the best thing to do was to reclassify the symbols so the local variables could be filtered later. Where could local variables be filtered?

Notice although we Rust users don't want to see local variables in the outline view, Javascript users may want them there. It could be useful to have local variables in the symbol search as well.

@alexheretic
Copy link
Member

alexheretic commented Feb 28, 2018

It seems that the outline in atom uses the documentSymbol containerName to build a parent/child relation in the outline view. Rls doesn't supply this so it's totally flat. I'm not sure how useful this outline is without the parent/child thing.

For example this is how it looks for ide-typescript

I haven't been following the discussion too closely though, do you know why we aren't implementing containerName usage in Rls?

But if you want to try filtering client-side, override atom-languageclient's behaviour with something like this in ide-rust

// index.js
class RustLanguageClient extends AutoLanguageClient {
  ...
  provideOutlines() {
    let provide = super.provideOutlines()
    let superOutline = provide.getOutline

    provide.getOutline = async (...args) => {
      let outline = await superOutline.apply(this, args)
      outline.outlineTrees = outline.outlineTrees
        .filter(o => o.icon !== "type-variable")
      return outline
    }

    return provide
  }
}

It looks to me like the outline isn't very useful with or without the filtering.

@aldonogueira
Copy link
Contributor Author

Even though the outline view would be more useful with hierarchy, the items are in the correct order. I've seen the outline with no local variables when I tested my version of RLS. It's much better than what we have now.

I know there are some difficulties determining proper hierarchy of Rust elements, but the last entry on that issue is from 23 Nov 2016. :( It looks like, since vscode doesn't have an outline view, the people that develops RLS don't think of this feature as an priority.

Thanks for the code sample. I'll try filtering with my version of RLS.

@aldonogueira
Copy link
Contributor Author

What happened to the icons in outline view? I have just checked my Atom and they are gone here too for Java, Rust and Javascript :(

@aldonogueira
Copy link
Contributor Author

Nevermind. I found it: facebookarchive/atom-ide-ui#175

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants