nano-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Nano-devel] [PATCH] Implement pipe to command feature in nano


From: Benno Schulenberg
Subject: Re: [Nano-devel] [PATCH] Implement pipe to command feature in nano
Date: Wed, 16 May 2018 20:52:04 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.7.0

Op 16-05-18 om 03:31 schreef Marco Diego Aurélio Mesquita:
> The attached patch implements the pipe to external command feature in
> nano. Differences from previous version are:
>   - merged Benno's suggestions and
>   - fixed statusbar position when toggling the feature.
> 
> Now that we just had a new release, I think it is a good to merged
> such an anticipated feature. Hope this version is good to go now.

You're still in a hurry, hm?  :)

> +                     if (func == flip_pipe) {
> +                             if (answer && answer[0] == '|') {
> +                                     char *aux = mallocstrcpy(NULL, 
> &answer[1]);
> +                                     free(answer);
> +                                     answer = aux;
> +                                     statusbar_x--;

You shouldn't decrease statusbar_x when it is zero.

Also, now I remember that the rest of nano uses charmove() to add
or remove something from a piece of text.  So I've updated the patch
to do that.  Coming up.

> -static size_t statusbar_x = HIGHEST_POSITIVE;
> +size_t statusbar_x = HIGHEST_POSITIVE;
>               /* The cursor position in answer. */

All other global vars are declared in global.c, so I've moved
it there.

> +/* Sends buffer pointed to by inptr to file descriptor fd. */
> +void send_data(const filestruct *line, int fd)

Comments should match the code.  Fixed.

> +     /* If text was selected, pipe it to external command. */
> +     if (should_pipe) {

Again, comment should match code.

> +     /* Before we start reading the forked command's output, we set
> +      * things up so that Ctrl-C will cancel the new process. */

Don't add back stuff that I deleted.  (Don't just look at the
code, but also look at the patch that git format-patch produces,
and make sure that every line makes sense.)

The patch looks good now.  But it won't go in until a subsequent
patch is available that undoes the text replacement in a single
step.  This shouldn't be too hard.  When I find some time in the
coming days, I will work on it.

Benno

Attachment: signature.asc
Description: OpenPGP digital signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]