|
From: | Marco Diego Aurélio Mesquita |
Subject: | Re: [Nano-devel] [PATCH 1/2] new feature: pipe selected text to external command when executing one |
Date: | Fri, 11 May 2018 16:11:56 -0300 |
Just a small oversight: we don't need to care about statusbar_x anymore. Attached patchset should be good. Thanks! On Fri, May 11, 2018 at 3:51 PM, Marco Diego Aurélio Mesquita <address@hidden> wrote: > On Fri, May 11, 2018 at 1:30 PM, Benno Schulenberg <address@hidden> wrote: >> >> Hello Marco, > > Hi Benno! > >> >> 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. >> > > Good! > >> 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. >> > > Fixed on the attached patch set. > >>> I really would like to get this feature in. >> >> It won't make it into 2.9.7, that's for sure. >> > > Ok. > >>> Is the undo thing really >>> necessary for now? Can't it be done later? >> >> Maybe. >> > > Hope so. > >>> + fprintf(stderr, "First char is a >>> pipe, we should remove it.\n"); >> >> A debugging leftover? >> > > Yes. Removed on the attached patchset. > >>> + 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. >> > > Done. > >>> + 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. >> > > Done. > >>> + if (should_pipe) { >>> + if (ISSET(MULTIBUFFER)) >>> + switch_to_prev_buffer(); >> >> Why do you need to change buffer? What if there is no other buffer? >> > > To allow the output of the command to go to another buffer. This gives > the user the flexibility of marking some part of the text, pipe it to > an external tool and have its output in a new 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. >> > > Fixed. > >>> @@ -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. >> > > Fixed. > > Thanks to the review. Hope it is good enough now.
0001-Pipe-slected-text-to-external-command.patch
Description: Text Data
0004-Minor-improvements-to-pipe-to-external-tool-feature.patch
Description: Text Data
0003-Reshuffle-some-code-and-improve-some-variable-names.patch
Description: Text Data
0002-Use-in-the-first-char-at-a-command-to-determine-if-t.patch
Description: Text Data
[Prev in Thread] | Current Thread | [Next in Thread] |