Skip to content

Conversation

@popzxc
Copy link
Contributor

@popzxc popzxc commented Oct 24, 2020

Continuing implementing suggestions from the Completion refactoring zulip thread.

This PR does the following:

  • Removes dependency of completions on assists by moving required functionality into ide_db.
  • Moves completely call_info crate into ide_db as it looks like it fits perfect there.
  • Adds a bunch of new tests and docs.
  • Adds the re-export of base_db to the ide_db and removes direct dependency on base_db from other crates.

The last point is controversial, I guess, but I noticed that in places where ide_db is used, base_db is also always used. Thus I think the dependency on the base_db is implied by the fact of ide_db interfaces, and thus it makes sense to just provide base_db out of the box.

AssistContext, AssistId, AssistKind, Assists,
};
use crate::{utils::unwrap_trivial_block, AssistContext, AssistId, AssistKind, Assists};
use ide_db::ty_filter::TryEnum;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very minor and is not required to be fixed, but we put crate:: and other_crate:: deps in different groups: https://github.com/rust-analyzer/rust-analyzer/blob/master/docs/dev/style.md#order-of-imports

Copy link
Contributor

@matklad matklad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

bors r+
bors d+

(in case this conflicts with another .lock changing PR in queue)

expect![[r#""#]],
);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

use crate::{line_index::LineIndex, symbol_index::SymbolsDatabase};

/// `base_db` is normally also needed in places where `ide_db` is used, so this re-export is for convenience.
pub use base_db;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@bors
Copy link
Contributor

bors bot commented Oct 24, 2020

✌️ popzxc can now approve this pull request. To approve and merge a pull request, simply reply with bors r+. More detailed instructions are available here.

@bors
Copy link
Contributor

bors bot commented Oct 24, 2020

@bors bors bot merged commit bf84e49 into rust-lang:master Oct 24, 2020
@popzxc popzxc deleted the short-dependency-chain branch October 25, 2020 03:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants