From: Florian Obser <florian@openbsd.org>
Subject: Re: mg: move up directory in dired
To: Omar Polo <op@omarpolo.com>
Cc: Dante Catalfamo <dante@lambda.cx>,  tech@openbsd.org
Date: Mon, 03 Jun 2024 22:03:04 +0200

On 2024-06-03 19:05 +02, Omar Polo <op@omarpolo.com> wrote:
> On 2024/06/02 02:12:57 -0400, Dante Catalfamo <dante@lambda.cx> wrote:
>> I had forgotten to add the command to the function map
>
> Looks great to me!  I've missed something like this to jump to the
> parent directory quickly than searching for the '..' entry.
>
> I have a few nits, mostly on the style, but if noone complains I'll
> commit in a few days.

OK florian

>
>> [...]
>> +int
>> +d_updirectory(int f, int n)
>> +{
>> +
>> +	struct buffer *bp;
>> +	int ret;
>> +	char  fname[NFILEN];
>
> nit: I'd keep these aligned as in the rest of the file.
>
>> +	ret = snprintf(fname, sizeof(fname), "%s..", curbp->b_fname);
>> +	if (ret < 0 || ret >= (int)sizeof(fname))
>
> nit/cargo culting: to stay closer to the "proper secure idiom" outlined
> in printf(3) CAVEATS section, I'd change the casting to
>
> 	if (ret < 0 || (size_t)ret >= sizeof(buff))
>
> even if in practice it's exactly the same stuff.
>
>> +		return (ABORT); /* Name is too long. */
>> +
>> +	bp = dired_(fname);
>> +	if (bp == NULL)
>> +		return (FALSE);
>> +	curbp = bp;
>> +	if (showbuffer(bp, curwp, WFFULL) != TRUE)
>> +		return (FALSE);
>> +	if (bp->b_fname[0] != 0)
>> +		return (TRUE);
>> +	return (readin(fname));
>> +}
>
> Then I also think we should document this addition in mg.1.  I'm
> attaching an updated diff with the manpage addition, plus a proposed
> follow-up
>
> diff -s /usr/src
> commit - 47c8d1014851805fa59b884ed3dceb148358575f
> path + /usr/src (staged changes)
> blob - 579ac9a124f4e66007c8dcdd88d2c8f10aad71d9
> blob + 2fcb1c49f374b02c58136c3267d7171acfd2655e
> --- usr.bin/mg/dired.c
> +++ usr.bin/mg/dired.c
> @@ -34,6 +34,7 @@ static int	 d_otherwindow(int, int);
>  static int	 d_undel(int, int);
>  static int	 d_undelbak(int, int);
>  static int	 d_findfile(int, int);
> +static int	 d_updirectory(int, int);
>  static int	 d_ffotherwindow(int, int);
>  static int	 d_expunge(int, int);
>  static int	 d_copy(int, int);
> @@ -122,6 +123,10 @@ static PF diredcz[] = {
>  	d_create_directory	/* + */
>  };
>  
> +static PF diredcaret[] = {
> +	d_updirectory /* ^ */
> +};
> +
>  static PF direda[] = {
>  	d_filevisitalt,		/* a */
>  	rescan,			/* b */
> @@ -172,9 +177,9 @@ static struct KEYMAPE (1) d_backpagemap = {
>  	}
>  };
>  
> -static struct KEYMAPE (7) diredmap = {
> -	7,
> -	7,
> +static struct KEYMAPE (8) diredmap = {
> +	8,
> +	8,
>  	rescan,
>  	{
>  		{
> @@ -191,6 +196,9 @@ static struct KEYMAPE (7) diredmap = {
>  			CCHR('Z'), '+', diredcz, (KEYMAP *) & metamap
>  		},
>  		{
> +			'^', '^', diredcaret, NULL
> +		},
> +		{
>  			'a', 'j', direda, NULL
>  		},
>  		{
> @@ -224,6 +232,7 @@ dired_init(void)
>  	funmap_add(d_undel, "dired-unmark", 0);
>  	funmap_add(d_undelbak, "dired-unmark-backward", 0);
>  	funmap_add(d_killbuffer_cmd, "quit-window", 0);
> +	funmap_add(d_updirectory, "dired-up-directory", 0);
>  	maps_add((KEYMAP *)&diredmap, "dired");
>  	dobindkey(fundamental_map, "dired", "^Xd");
>  }
> @@ -364,6 +373,28 @@ d_findfile(int f, int n)
>  }
>  
>  int
> +d_updirectory(int f, int n)
> +{
> +	struct buffer	*bp;
> +	int		 ret;
> +	char		 fname[NFILEN];
> +
> +	ret = snprintf(fname, sizeof(fname), "%s..", curbp->b_fname);
> +	if (ret < 0 || (size_t)ret >= sizeof(fname))
> +		return (ABORT); /* Name is too long. */
> +
> +	bp = dired_(fname);
> +	if (bp == NULL)
> +		return (FALSE);
> +	curbp = bp;
> +	if (showbuffer(bp, curwp, WFFULL) != TRUE)
> +		return (FALSE);
> +	if (bp->b_fname[0] != 0)
> +		return (TRUE);
> +	return (readin(fname));
> +}
> +
> +int
>  d_ffotherwindow(int f, int n)
>  {
>  	char		 fname[NFILEN];
> blob - 4faeb8fb293954d061e883a8a53482743afcde3b
> blob + 366d6087064a44bb3d28a750e3a7d4a8258adc5f
> --- usr.bin/mg/mg.1
> +++ usr.bin/mg/mg.1
> @@ -1055,6 +1055,8 @@ dired-next-line
>  dired-shell-command
>  .It +
>  dired-create-directory
> +.It ^
> +dired-up-directory
>  .It a
>  dired-find-alternate-file
>  .It c
> @@ -1130,6 +1132,8 @@ Remove the deletion flag for the file on the current l
>  .It Ic dired-unmark-backward
>  Remove the deletion flag from the file listed on the previous line
>  of the dired buffer, then move up to that line.
> +.It Ic dired-up-directory
> +Open a dired buffer in the parent directory.
>  .It Ic quit-window
>  Close the current dired buffer.
>  .El
>
>
> Then, since we have dired-up-directory, I'd propose to use it in
> dired-jump since it's handy to keep pressing C-x C-j to navigate
> upwards.  GNU Emacs does this by default too.
>
> diff /usr/src
> commit - 47c8d1014851805fa59b884ed3dceb148358575f
> path + /usr/src
> blob - 2fcb1c49f374b02c58136c3267d7171acfd2655e (staged)
> file + usr.bin/mg/dired.c
> --- usr.bin/mg/dired.c
> +++ usr.bin/mg/dired.c
> @@ -1125,7 +1125,7 @@ dired_jump(int f, int n)
>  	for (i = 0; i <= curbp->b_nmodes; i++) {
>  		modename = curbp->b_modes[i]->p_name;
>  		if (strncmp(modename, "dired", 5) == 0)
> -			return (dobeep_msg("In dired mode already"));
> +			return (d_updirectory(f, n));
>  	}
>  
>  	if (getbufcwd(dname, sizeof(dname)) != TRUE)
>
>
> Thanks!
>

-- 
In my defence, I have been left unsupervised.