Default value for socket?

[tfheen]

Could socket by default just live in $XDG_RUNTIME_DIR/rkeep.sock (or similar), if $XDG_RUNTIME_DIR is set?

[leaty]

I always advocate for standards, so yes I agree this should be done. I think $XDG_RUNTIME_DIR/rkeep/rkeep.sock to align with the rest is best?

As for having it as default, which of these make the most sense?

1. Supporting $XDG_RUNTIME_DIR/rkeep/rkeep.sock in the config.
2. Uses $XDG_RUNTIME_DIR/rkeep/rkeep.sock if socket is omitted from the config file.

I think 1, because it also allows customizing the default path. It also makes it more clear in the example config.

[tfheen]

Making it live in a subdirectory is fine; I don't have any strong opinions on exactly what the default should be (but then rkeepd needs to create the directory if it doesn't exist).

I have a mild preference for 2 so most users won't have to care about the socket at all, but allowing users to override the setting if they for some reason need that. That said, you could do both?

[leaty]

Makes sense, I suppose a comment on this in the config would be enough to explain the omittance. Is this something you would like to PR?

[tfheen]

I can take a look at this, but to make it non-ugly, I think introducing a configuration parsing crate makes sense. I don't have a preference, do you have any, or should I just look at what's out there? It would be nice to not have the duplication of logic in server.rs and client.rs.

(The ugly way to do this is something like:

diff --git src/client.rs src/client.rs
index 2432a8a..cc4486d 100644
--- src/client.rs
+++ src/client.rs
@@ -34,11 +34,15 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
 	// Load config
 	let config_file = args.value_of("c").unwrap_or(&default);
 	let config_str = fs::read_to_string(&config_file)?;
-	let config: Config = toml::from_str(&config_str).unwrap();
+	let mut config: Config = toml::from_str(&config_str).unwrap();
+
+	if config.socket.is_none() {
+		config.socket = Some(xdgd.place_runtime_file("rkeep.sock").unwrap());
+	}
 
 	let session = args.value_of("s").unwrap();
 
-	let mut stream = UnixStream::connect(config.socket)?;
+	let mut stream = UnixStream::connect(config.socket.unwrap())?;
 	stream.write_all(format!("{}|exec", session).as_bytes())?;
 
 	Ok(())
diff --git src/config.rs src/config.rs
index 3985526..5c1e3b1 100644
--- src/config.rs
+++ src/config.rs
@@ -3,7 +3,7 @@ use std::path::PathBuf;
 
 #[derive(Deserialize)]
 pub struct Config {
-	pub socket: PathBuf,
+	pub socket: Option<PathBuf>,
 	pub session: Vec<Session>,
 }
 
diff --git src/server.rs src/server.rs
index 0f3fb50..47ebeda 100644
--- src/server.rs
+++ src/server.rs
@@ -38,7 +38,11 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
 	// Load config
 	let config_file = args.value_of("c").unwrap_or(&default);
 	let config_str = fs::read_to_string(&config_file)?;
-	let mut config: Config = toml::from_str(&config_str).unwrap();
+	let mut config: Config = toml::from_str(&config_str).expect("Invalid toml");
+
+	if config.socket.is_none() {
+		config.socket = Some(xdgd.place_runtime_file("rkeep.sock").unwrap());
+	}
 
 	// Set up sessions
 	let sessions = Arc::new(Mutex::new(HashMap::<String, keepass::Session>::new()));
@@ -54,8 +58,8 @@ fn main() -> Result<(), Box<dyn std::error::Error>> {
 	cleaning(Arc::clone(&sessions));
 
 	// Start listening for clients
-	let _ = fs::remove_file(&config.socket);
-	let listener = UnixListener::bind(config.socket)?;
+	let _ = fs::remove_file(&config.socket.clone().unwrap());
+	let listener = UnixListener::bind(config.socket.unwrap())?;
 	for stream in listener.incoming() {
 		match stream {
 			Ok(stream) => {

)

[leaty]

I haven't really used any configuration parsing crates so I actually don't know if it's cleaner or not. I figured serde is automated enough, but I see what you're saying with the WET if on socket in client/server that happens with this change.

All we need is a nice hook to run the shellexpand crate on socket/database paths after deserializing. But implementing a whole serde deserialize is kinda tedious for something like this, unfortunately serde-rs/serde#642 isn't on the cards yet, but it would've fit perfectly here, there is however a workaround in the last 2 comments that look interesting.

Otherwise I'm actually just thinking having a parse fn in Config impl so you can run config.parse(); in both client and server, and it just modifies the paths using shellexpand. Not perfect, but good enough to me. Alternatively you could make the path fields private and just add functions which return a shellexpanded version, but it's kinda shitty if you have to fetch it more than once.

This is all assuming a configuration crate isn't WAY easier though.

EDIT: If you think a configuration crate is cleaner, go for it.