Suggestion: make iwctl follow the XDG Base Directory Specification

Suggestion: make iwctl follow the XDG Base Directory Specification

[hoe.dom]

[-- Attachment #1: Type: text/plain, Size: 613 bytes --]

Hi.
Currently iwctl writes its history to a file named .iwctl_history in the users home directory. The XDG Base Directory Specification tries to standardize these kind of things (see https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html for details). The bottom line would probably be to:
* check XDG_DATA_HOME (with fall back to .local/share/) and save the history in an iwd subfolder of that directory
* potentially read (and migrate?) .iwctl_history from the home directory for compatibility purposes

Thanks for iwd by the way. Its a huge improvement over the wpa_supplicant world!

Re: Suggestion: make iwctl follow the XDG Base Directory Specification

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 727 bytes --]

Hi,

> Currently iwctl writes its history to a file named .iwctl_history in the users home directory. The XDG Base Directory Specification tries to standardize these kind of things (see https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html for details). The bottom line would probably be to:
> * check XDG_DATA_HOME (with fall back to .local/share/) and save the history in an iwd subfolder of that directory
> * potentially read (and migrate?) .iwctl_history from the home directory for compatibility purposes

I am fine with using $XDG_DATA_HOME/iwctl/history or $HOME/.local/share/iwctl/history as fallback if $XDG_DATA_HOME is not provided. Care to send a patch for this?

Regards

Marcel

Re: Suggestion: make iwctl follow the XDG Base Directory Specification

[Xaver =?unknown-8bit?q?H=C3=B6rl?=]

[-- Attachment #1: Type: text/plain, Size: 945 bytes --]

Sure I will give it a try. Note though that I don't usually code in C
and have no experience contributing to this sort of stuff.

On Sat, Dec 07, 2019 at 04:47:39PM +0100, Marcel Holtmann wrote:
> Hi,
>
> > Currently iwctl writes its history to a file named .iwctl_history in the users home directory. The XDG Base Directory Specification tries to standardize these kind of things (see https://specifications.freedesktop.org/basedir-spec/basedir-spec-latest.html for details). The bottom line would probably be to:
> > * check XDG_DATA_HOME (with fall back to .local/share/) and save the history in an iwd subfolder of that directory
> > * potentially read (and migrate?) .iwctl_history from the home directory for compatibility purposes
>
> I am fine with using $XDG_DATA_HOME/iwctl/history or $HOME/.local/share/iwctl/history as fallback if $XDG_DATA_HOME is not provided. Care to send a patch for this?
>
> Regards
>
> Marcel
>

[PATCH] client: Follow the xdg base directory specification

[Xaver =?unknown-8bit?q?H=C3=B6rl?=]

[-- Attachment #1: Type: text/plain, Size: 2458 bytes --]

Use XDG_DATA_HOME as location for the history file.
Also reads and migrates the history from the old location in the users
home directory if present.
---
 client/display.c | 68 +++++++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 62 insertions(+), 6 deletions(-)

diff --git a/client/display.c b/client/display.c
index 10c87b2a..d1a1a7f7 100644
--- a/client/display.c
+++ b/client/display.c
@@ -27,6 +27,9 @@
 #define _GNU_SOURCE
 #include <stdio.h>
 #include <signal.h>
+#include <unistd.h>
+#include <sys/types.h>
+#include <sys/stat.h>

 #include <readline/history.h>
 #include <readline/readline.h>
@@ -632,22 +635,75 @@ static void signal_handler(void *user_data)
 		display_refresh_reset();
 }

+char * get_data_dir()
+{
+	const char *xdg_data_home;
+	const char *home_path;
+	char *data_dir;
+
+	xdg_data_home = getenv("XDG_DATA_HOME");
+	if (!xdg_data_home || *xdg_data_home != '/') {
+		home_path = getenv("HOME");
+		if (home_path)
+			data_dir = l_strdup_printf("%s/%s", home_path,
+						".local/share/iwctl");
+		else
+			return NULL;
+	} else {
+		data_dir = l_strdup_printf("%s/%s", xdg_data_home,
+							"iwctl");
+	}
+	return data_dir;
+}
+
+char * get_old_history_file()
+{
+	const char *home_path;
+	char *old_history_path;
+	struct stat st;
+	int err;
+
+	home_path = getenv("HOME");
+	if (home_path) {
+		old_history_path = l_strdup_printf("%s/%s", home_path,
+						".iwctl_history");
+
+		err = stat(old_history_path, &st);
+		if (err || !S_ISREG(st.st_mode)) {
+			l_free(old_history_path);
+			return NULL;
+		}
+	}
+	return old_history_path;
+}
+
 static char *history_path;

 void display_init(void)
 {
-	const char *home_path;
+	char *data_dir;
+	char *old_history_file;

 	display_refresh.redo_entries = l_queue_new();

 	stifle_history(24);

-	home_path = getenv("HOME");
-	if (home_path)
-		history_path = l_strdup_printf("%s/%s", home_path,
-							".iwctl_history");
+	data_dir = get_data_dir();
+	if (data_dir) {
+		mkdir(data_dir,0700);
+
+		history_path = l_strdup_printf("%s/%s", data_dir,
+							"history");
+		l_free(data_dir);
+	}

-	read_history(history_path);
+	old_history_file = get_old_history_file();
+	if (old_history_file) {
+		read_history(old_history_file);
+		unlink(old_history_file);
+		l_free(old_history_file);
+	} else
+		read_history(history_path);

 	setlinebuf(stdout);

--
2.24.0

Re: [PATCH] client: Follow the xdg base directory specification

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 2798 bytes --]

Hi Xaver,

> Use XDG_DATA_HOME as location for the history file.
> Also reads and migrates the history from the old location in the users
> home directory if present.
> ---
> client/display.c | 68 +++++++++++++++++++++++++++++++++++++++++++-----
> 1 file changed, 62 insertions(+), 6 deletions(-)
> 
> diff --git a/client/display.c b/client/display.c
> index 10c87b2a..d1a1a7f7 100644
> --- a/client/display.c
> +++ b/client/display.c
> @@ -27,6 +27,9 @@
> #define _GNU_SOURCE
> #include <stdio.h>
> #include <signal.h>
> +#include <unistd.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> 
> #include <readline/history.h>
> #include <readline/readline.h>
> @@ -632,22 +635,75 @@ static void signal_handler(void *user_data)
> 		display_refresh_reset();
> }
> 
> +char * get_data_dir()
> +{
> +	const char *xdg_data_home;
> +	const char *home_path;
> +	char *data_dir;
> +
> +	xdg_data_home = getenv("XDG_DATA_HOME");
> +	if (!xdg_data_home || *xdg_data_home != '/') {
> +		home_path = getenv("HOME");
> +		if (home_path)
> +			data_dir = l_strdup_printf("%s/%s", home_path,
> +						".local/share/iwctl");
> +		else
> +			return NULL;
> +	} else {
> +		data_dir = l_strdup_printf("%s/%s", xdg_data_home,
> +							"iwctl");
> +	}
> +	return data_dir;
> +}
> +
> +char * get_old_history_file()
> +{
> +	const char *home_path;
> +	char *old_history_path;
> +	struct stat st;
> +	int err;
> +
> +	home_path = getenv("HOME");
> +	if (home_path) {
> +		old_history_path = l_strdup_printf("%s/%s", home_path,
> +						".iwctl_history");
> +
> +		err = stat(old_history_path, &st);
> +		if (err || !S_ISREG(st.st_mode)) {
> +			l_free(old_history_path);
> +			return NULL;
> +		}
> +	}
> +	return old_history_path;
> +}
> +
> static char *history_path;
> 
> void display_init(void)
> {
> -	const char *home_path;
> +	char *data_dir;
> +	char *old_history_file;
> 
> 	display_refresh.redo_entries = l_queue_new();
> 
> 	stifle_history(24);
> 
> -	home_path = getenv("HOME");
> -	if (home_path)
> -		history_path = l_strdup_printf("%s/%s", home_path,
> -							".iwctl_history");
> +	data_dir = get_data_dir();
> +	if (data_dir) {
> +		mkdir(data_dir,0700);
> +
> +		history_path = l_strdup_printf("%s/%s", data_dir,
> +							"history");
> +		l_free(data_dir);
> +	}
> 
> -	read_history(history_path);
> +	old_history_file = get_old_history_file();
> +	if (old_history_file) {
> +		read_history(old_history_file);
> +		unlink(old_history_file);
> +		l_free(old_history_file);
> +	} else
> +		read_history(history_path);

I am not sure to bother with the old .iwctl_history file. For now I have created and pushed a patch that moves this to iwctl/history under XDG_DATA_HOME.

Regards

Marcel