Skip to content
This repository has been archived by the owner on Dec 29, 2022. It is now read-only.

Use Inferrable for target/ config option #793

Merged
merged 5 commits into from
Mar 29, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/build/cargo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ fn run_cargo(
let config = {
let rls_config = rls_config.lock().unwrap();

let target_dir = rls_config.target_dir.as_ref().map(|p| p as &Path);
let target_dir = rls_config.target_dir.as_ref().as_ref().map(|p| p as &Path);
make_cargo_config(manifest_dir, target_dir, restore_env.get_old_cwd(), shell)
};

Expand Down
36 changes: 28 additions & 8 deletions src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,16 @@ impl<'de, T: Deserialize<'de>> Deserialize<'de> for Inferrable<T> {
}
}

impl<T> Inferrable<T> {
pub fn is_none(&self) -> bool {
if let &Inferrable::None = self {
return false;
} else {
return true;
}
Copy link
Member

Choose a reason for hiding this comment

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

This would be better as a match

Copy link
Member Author

Choose a reason for hiding this comment

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

Done. Let's, uh, not talk about previous, totally correct return false, though!

}
}

impl<T: Clone + Debug> Inferrable<T> {
/// Combine these inferrable values, preferring our own specified values
/// when possible, and falling back the given default value.
Expand Down Expand Up @@ -104,6 +114,7 @@ impl<T> AsRef<T> for Inferrable<T> {
}
}
}

/// RLS configuration options.
#[derive(Clone, Debug, Deserialize, Serialize)]
#[allow(missing_docs)]
Expand All @@ -126,8 +137,7 @@ pub struct Config {
pub build_on_save: bool,
pub use_crate_blacklist: bool,
/// Cargo target dir. If set overrides the default one.
#[serde(skip_deserializing, skip_serializing)]
pub target_dir: Option<PathBuf>,
pub target_dir: Inferrable<Option<PathBuf>>,
pub features: Vec<String>,
pub all_features: bool,
pub no_default_features: bool,
Expand Down Expand Up @@ -156,7 +166,7 @@ impl Default for Config {
clear_env_rust_log: true,
build_on_save: false,
use_crate_blacklist: true,
target_dir: None,
target_dir: Inferrable::Inferred(None),
features: vec![],
all_features: false,
no_default_features: false,
Expand All @@ -173,6 +183,7 @@ impl Default for Config {
impl Config {
/// Join this configuration with the new config.
pub fn update(&mut self, mut new: Config) {
new.target_dir = self.target_dir.combine_with_default(&new.target_dir, None);
new.build_lib = self.build_lib.combine_with_default(&new.build_lib, false);
new.build_bin = self.build_bin.combine_with_default(&new.build_bin, None);

Expand Down Expand Up @@ -203,10 +214,11 @@ impl Config {

/// Is this config incomplete, and needs additional values to be inferred?
pub fn needs_inference(&self) -> bool {
match (&self.build_lib, &self.build_bin) {
(&Inferrable::None, _) | (_, &Inferrable::None) => true,
_ => false,
if self.build_bin.is_none() || self.build_lib.is_none() || self.target_dir.is_none() {
return true;
}

return true;
Copy link
Member

Choose a reason for hiding this comment

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

This looks wrong - it's essentially if ... { true } else { true }

Copy link
Member

Choose a reason for hiding this comment

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

And you don't need the final return

}

/// Tries to auto-detect certain option values if they were unspecified.
Expand All @@ -232,13 +244,21 @@ impl Config {
// Constructing a `Workspace` also probes the filesystem and detects where to place the
// build artifacts. We need to rely on Cargo's behaviour directly not to possibly place our
// own artifacts somewhere else (e.g. when analyzing only a single crate in a workspace)
if self.target_dir.is_none() {
match self.target_dir {
// We require an absolute path, so adjust a relative one if it's passed.
Inferrable::Specified(Some(ref mut path)) if path.is_relative() => {
*path = project_dir.join(&path);
}
_ => {},
}
if self.target_dir.as_ref().is_none() {
let target_dir = ws.target_dir().clone().into_path_unlocked();
let target_dir = target_dir.join("rls");
self.target_dir.infer(Some(target_dir));
trace!(
"For project path {:?} Cargo told us to use this target/ dir: {:?}",
project_dir,
self.target_dir.as_ref().unwrap(),
self.target_dir.as_ref().as_ref().unwrap(),
);
}

Expand Down