Should (icomplete-mode) explicitly disable fido-mode?

Should (icomplete-mode) explicitly disable fido-mode?

[Andrew Schwartzmeyer]

[-- Attachment #1: Type: text/plain, Size: 1231 bytes --]

Hi João,

I was wondering if this bit of code in icomplete-mode needs to be changed:

  (when icomplete-mode
    (fido-mode -1)...

Running (icomplete-mode) explicitly disables fido-mode, whereas running (fido-mode) makes it appear that icomplete-mode is enabled (sets it to t and runs its setup, without actually running the function icomplete-mode).

This causes third party packages which wrap icomplete-mode to easily break fido-mode, like in this issue: https://github.com/oantolin/icomplete-vertical/pull/13 <https://github.com/oantolin/icomplete-vertical/pull/13>

icomplete-vertical calls (icomplete-mode), I think expecting it to be idempotent (not have any effect if icomplete-mode is already enabled), but in fact, since fido-mode is currently enabled this explicitly disables it and switches to the base icomplete-mode, as the mode functions icomplete-mode and fido-mode act like toggles of each other.

Should (icomplete-mode t) perhaps do nothing if fido-mode is already enabled? I’m not sure. Obviously the package can workaround it (given my patch), but I think it’s unexpected to have to avoid calling (icomplete-mode) if the icomplete-mode var is already t. What do you think?

Thanks,

Andy

[-- Attachment #2: Type: text/html, Size: 1921 bytes --]

Re: Should (icomplete-mode) explicitly disable fido-mode?

[João Távora]

Andrew Schwartzmeyer <andrew@schwartzmeyer.com> writes:

> icomplete-vertical calls (icomplete-mode), I think expecting it to be
> idempotent (not have any effect if icomplete-mode is already enabled),
> but in fact, since fido-mode is currently enabled this explicitly
> disables it and switches to the base icomplete-mode, as the mode
> functions icomplete-mode and fido-mode act like toggles of each
> other.

But isn't the end result what icomplete-vertical is after?
i.e. icomplete with some kind of verticality added?  Or were you
expecting icomplete-vertical to add verticality to fido-mode?

> Should (icomplete-mode t) perhaps do nothing if fido-mode is already
> enabled?

Maybe.

> I’m not sure. Obviously the package can workaround it (given
> my patch), but I think it’s unexpected to have to avoid calling
> (icomplete-mode) if the icomplete-mode var is already t. What do you
> think?

It's a tricky situation, indeed.  Fido-mode sets the icomplete-mode
variable for effects, i.e. to have some functions that check that
variable behave differently.  But tries not to "be" icomplete-mode.
Maybe Stefan Monnier would have something to say about this

João

Re: Should (icomplete-mode) explicitly disable fido-mode?

[Stefan Monnier]

> Maybe Stefan Monnier would have something to say about this

Probably not ;-)
Various behaviors make sense and I don't have a strong opinion about
what should happen.

The only thing that might be worth investigating is why
icomplete-vertical needs to call icomplete-mode when it's
already enabled.  Maybe we could arrange for this not to be needed at
all and avoid the problem.


        Stefan

Re: Should (icomplete-mode) explicitly disable fido-mode?

[Andrew Schwartzmeyer]

[-- Attachment #1: Type: text/plain, Size: 1271 bytes --]

On Jun 5, 2020, at 3:48 PM, João Távora <joaotavora@gmail.com> wrote:
> 
> But isn't the end result what icomplete-vertical is after?
> i.e. icomplete with some kind of verticality added?  Or were you
> expecting icomplete-vertical to add verticality to fido-mode?

Since fido-mode is just icomplete-mode with nice defaults, icomplete-vertical just becomes fido-vertical. With that patch they took, it works like a charm. I get all the semantics and keybindings of fido-mode, and use the icomplete-vertical-do macro for my yank-pop+ and recentf-list+ functions based on completing-read (the content in these completions tables is better suited to a vertical representation, whereas most of the time I want the default horizontal representation).

> The only thing that might be worth investigating is why
> icomplete-vertical needs to call icomplete-mode when it's
> already enabled.  Maybe we could arrange for this not to be needed at
> all and avoid the problem.

I patched icomplete-vertical with (unless icomplete-mode (icomplete-mode)), but it felt like a work-around. I don’t know of any other instance where calling foo-mode non-interactively will break things if it’s already enabled. I thought they were usually idempotent.

Cheers,

Andy

[-- Attachment #2: Type: text/html, Size: 4234 bytes --]