Re: Possible new feature: Use insert to try to insert file on current buffer

[Marco Diego Aurélio Mesquita]

On 5/15/20, Benno Schulenberg <address@hidden> wrote:
>
> Op 14-05-2020 om 21:17 schreef Marco Diego Aurélio Mesquita:
>> I have some macros
>
> What do you mean with "macros"?  String binds?  Because nano can
> have at most one macro defined.
>

Right, string bind.

>> that use external tools to add text to the current
>> buffer on the current cursor position. Also, it is very common for me
>> to open files on new buffers. Once I open a file on a new buffer, my
>> macros suddenly stop working because the output of the external tool
>> will go to a new buffer instead of the current one.
>
> Yes, it would have been better if M-F (and M-N) were one-time toggles,
> having effect only during the current invocation of ^R and forgotten
> the next time -- just like M-B is in the ^W menu.  Maybe they should
> be made to work that way?
>

This goes against the way I use nano. It is very common for me to
start nano with some files loaded and, after a few edits, open other
files. Having to re-set the open on new buffer flag every time I want
to load a file would be awkward.

>> Considering the pressing Ins is already redundant because it has the
>> same effect of pressing ^R, I wrote the attached small patch.
>
> Are you using this patch yourself?  If yes, how long have you been
> running with it?
>

I'm not using it. I don't like to keep modified nano versions on every
machine I use. I try to keep it mainline as much as possible. But I
don't think this matters, it would definitely help the described case.

>> With it
>> applied, when Ins is pressed, it will try to insert on the current
>> buffer independent of the current state of the MULTIBUFFER flag.
>> Changes when in this "mode" are also ignored so that it will not mess
>> with the value of the MULTIBUFFER flag.
>>
>> Please consider applying it.
>>
>>
>> Date: Thu, 7 May 2020 15:03:34 -0300
>> Subject: [PATCH] Ignore ISSET(MULTIBUFFER) when "Ins" is pressed.
>
> The style of nano's headerlines in git is: a word followed by a colon,
> followed by a sentence without a period.  The sentence tries to speak
> the language of the user, not of the code.  How can I take your patches
> seriously if even after twenty patches you still don't know how to
> format a subject line of patch?
>

Sorry. I shouldn't have said "consider applying it". It is actually
more a proof of concept than something in a finished state. Should
have been "please comment/review it"

>> Content-Transfer-Encoding: 8bit
>
> Where is the commit message?  How could I even consider applying it?
>
>

Improved on the attached patch.

>> +/* Insert a file but trying to do it in the current buffer first. */
>
> "trying"?
>
>> +void do_inserthere_void(void)
>
> The name is not good.  I tend to read it as "do inser there",
> and the "_void" is superfluous.
>

Improved on the attached patch.

>> +    bool was_set = ISSET(MULTIBUFFER);
>
> In general: a blank line after declarations.
>

Improved on the attached patch.

>> +    if (was_set)
>> +            TOGGLE(MULTIBUFFER);
>
> Instead of TOGGLE() I would use UNSET().
> Then it's clearer what the effect is.
>
>> +    if (ISSET(MULTIBUFFER) != was_set)
>> +            TOGGLE(MULTIBUFFER);
>
> Your logic is strange.  Why not simply:
>
>   if (was_set)
>       SET(MULTIBUFFER);
>

Improved on the attached patch.

> If the patch were to be applied, it would also need a
> bindable function, and documentation.  But currently
> I'm not inclined to apply it.
>

Please reconsider it. This small change allows some very interesting
and useful string binds.

Differences from previous version are:
        * Renamed do_inserthere_void to do_insert_here
        * Improved logic of the do_insert_here function
        * Improved comments
        * Added "insertfilehere" bindable function
        * Added documentation
        * Improved commit message

I decided on the name "insertfilehere" because I saw no bindable
function using the '_' character. I actuatlly like insert_here better.

Also, I added the new option to the end of the main menu. I wouldn't
like to see two so similar functions so close on the main menu. The
bad of this decision is that the option appears ungrouped on the help
cheat sheet.


Thanks for reviewing.

0001-new-feature-temporarily-ignore-flag-to-read-to-new-b.patch
Description: Text Data