Re: [Nano-devel] [PATCH 1/2] new feature: pipe selected text to external command when executing one

[Benno Schulenberg]

Hello Marco,

Op 11-05-18 om 04:13 schreef Marco Diego Aurélio Mesquita:
> These patches improve somewhat on what we had. It uses two
> simultaneous processes to pipe in and out, so we won't stuck because
> of buffers; implements what was discussed about the '|' to enable
> "pipeing"

Yeah, I had forgotten about that.  Hmm... I thought implementation of
this would be simpler, but... for now it works.

What does not work correctly is that when nothing is marked, and we're
piping, the entire buffer should be replaced with the output of the
external command -- the output should not be inserted into it, which
kind of doubles the the buffer.

> I really would like to get this feature in.

It won't make it into 2.9.7, that's for sure.

> Is the undo thing really
> necessary for now? Can't it be done later?

Maybe.

> +                                     fprintf(stderr, "First char is a pipe, 
> we should remove it.\n");

A debugging leftover?

> +                             statusbar_x = strlenpt(answer) + 1;

This is not right.  If the cursor is somewhere in the middle of the command,
then toggling the pipe thing should leave the cursor on the exact same
character.

> +             add_to_sclist(MEXTCMD, "^\\", 0, flip_pipe, 0);

I don't like ^\ as a toggle.  The other toggles are with Meta.
See attached patch for other minor adjustments -- for example
to not change the position of the M-F toggle in the help lines.

> +     if (should_pipe) {
> +             if (ISSET(MULTIBUFFER))
> +                     switch_to_prev_buffer();

Why do you need to change buffer?  What if there is no other buffer?

> +             if(has_selection) {
> +             #ifndef NANO_TINY
> +                     add_undo(CUT);
> +             #endif
> +                     do_cut_text(ISSET(MULTIBUFFER), !has_selection);
> +             #ifndef NANO_TINY
> +                     update_undo(CUT);
> +             #endif
> +             }

Oof...  This looks like a duplication of something else.  And besides,
the #ifs end #endifs should be at the start of the line.

> @@ -476,7 +476,7 @@ size_t strlenpt(const char *text)
>  {
>       size_t span = 0;
>  
> -     while (*text != '\0')
> +     while (text && *text != '\0')
>               text += parse_mbchar(text, NULL, &span);

No, don't make any of the util functions slower.  Which should be easy,
because you don't need strlenpt() at all.

Benno

signature.asc
Description: OpenPGP digital signature