bug#41624: 28.0.50; wdired-change-to-wdired-mode sometimes infloops

bug#41624: 28.0.50; wdired-change-to-wdired-mode sometimes infloops

[Michael Heerdegen]

Hello,

here is a recipe for emacs -Q:

M-x find-dired ~/ RET -iname \*.pdf RET  (any find-dired buffer will do)

M-: (setq wdired-allow-to-change-permissions 'advanced) RET

M-x wdired-change-to-wdired-mode RET

  ==> infloop

TIA,

Michael.



In GNU Emacs 28.0.50 (build 102, x86_64-pc-linux-gnu, GTK+ Version 3.24.20, cairo version 1.16.0)
 of 2020-05-31 built on drachen
Repository revision: 61617ac7957e7f909086ed72ce1cd43701ecf18a
Repository branch: master
Windowing system distributor 'The X.Org Foundation', version 11.0.12008000
System Description: Debian GNU/Linux bullseye/sid

bug#41624: 28.0.50; wdired-change-to-wdired-mode sometimes infloops

[Lars Ingebrigtsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> here is a recipe for emacs -Q:
>
> M-x find-dired ~/ RET -iname \*.pdf RET  (any find-dired buffer will do)
>
> M-: (setq wdired-allow-to-change-permissions 'advanced) RET
>
> M-x wdired-change-to-wdired-mode RET
>
>   ==> infloop

This bug is still present on the trunk, but I'm not quite sure whether
this is supposed to be supported or not?  The find-dired buffers aren't
very much like normal dired buffers...  so should wdired just signal an
error here?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#41624: 28.0.50; wdired-change-to-wdired-mode sometimes infloops

[Michael Heerdegen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> > M-x find-dired ~/ RET -iname \*.pdf RET  (any find-dired buffer will do)
> >
> > M-: (setq wdired-allow-to-change-permissions 'advanced) RET
> >
> > M-x wdired-change-to-wdired-mode RET
> >
> >   ==> infloop
>
> This bug is still present on the trunk, but I'm not quite sure whether
> this is supposed to be supported or not?  The find-dired buffers aren't
> very much like normal dired buffers...  so should wdired just signal an
> error here?

I can't answer that, but I can tell that when I manually delete the last
lines of text and whitespace in such a buffer (saying "find finished
..."), it works as expected (though I'm getting a message saying that "1
rename actions failed").  But I'm definitely able to change permissions
with wdired then.  So it seems possible to make the feature work, the
infloop results from the unsuitable termination condition of the loop in
`wdired-preprocess-perms'.

Michael.

bug#41624: 28.0.50; wdired-change-to-wdired-mode sometimes infloops

[Stefan Kangas]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> This bug is still present on the trunk, but I'm not quite sure whether
> this is supposed to be supported or not?  The find-dired buffers aren't
> very much like normal dired buffers...  so should wdired just signal an
> error here?

I think it would be a very nice feature to have, though.

Does it ever work?  If so, perhaps we should signal an error only for
the cases where we know it doesn't?  If we can easily decide that...

If it never works, it's of course better to signal an error than
inflooping or doing something broken.

bug#41624: 28.0.50; wdired-change-to-wdired-mode sometimes infloops

[Lars Ingebrigtsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> I can't answer that, but I can tell that when I manually delete the last
> lines of text and whitespace in such a buffer (saying "find finished
> ..."), it works as expected (though I'm getting a message saying that "1
> rename actions failed").  But I'm definitely able to change permissions
> with wdired then.  So it seems possible to make the feature work, the
> infloop results from the unsuitable termination condition of the loop in
> `wdired-preprocess-perms'.

If even renaming doesn't work, then it does sound like this was never
supposed to be a command that's applicable to dired-find buffers, I
think?

On the other hand, if this did work, then it'd be pretty nice.  But it
sounds less like a bug fix and more like a new feature.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#41624: 28.0.50; wdired-change-to-wdired-mode sometimes infloops

[Lars Ingebrigtsen]

Stefan Kangas <stefankangas@gmail.com> writes:

> If it never works, it's of course better to signal an error than
> inflooping or doing something broken.

Yes, I'm leaning towards signalling an error here.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#41624: 28.0.50; wdired-change-to-wdired-mode sometimes infloops

[Michael Heerdegen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> > I can't answer that, but I can tell that when I manually delete the last
> > lines of text and whitespace in such a buffer (saying "find finished
> > ..."), it works as expected (though I'm getting a message saying that "1
> > rename actions failed").  But I'm definitely able to change permissions
> > with wdired then.  So it seems possible to make the feature work, the
> > infloop results from the unsuitable termination condition of the loop in
> > `wdired-preprocess-perms'.
>
> If even renaming doesn't work, then it does sound like this was never
> supposed to be a command that's applicable to dired-find buffers, I
> think?

I didn't say renaming doesn't work, I only got that inappropriate
message.  Maybe just because I had edited the buffer by hand to make the
loop terminate.

I'm not convinced that this is not an intended use case. - on the
contrary.  Everything works, basically, it just seems that some details
in the code are not prepared for some things that are slightly
different, e.g. the final line in those buffers.

Please don't just signal an error unless you are really sure that this
never worked and was never intended to work, and is not easy to fix.

Regards,

Michael.

bug#41624: 28.0.50; wdired-change-to-wdired-mode sometimes infloops

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> I didn't say renaming doesn't work, I only got that inappropriate
> message.  Maybe just because I had edited the buffer by hand to make the
> loop terminate.

I found out that the infloop can be fixed easily if we change
`find-dired-sentinel' to let the buffer end in a newline.

Then I can change permissions with wdired, I only still get the
inappropriate message that it would have failed, though it has
succeeded.

Michael.

bug#41624: 28.0.50; wdired-change-to-wdired-mode sometimes infloops

[Michael Heerdegen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> Then I can change permissions with wdired, I only still get the
> inappropriate message that it would have failed, though it has
> succeeded.

Hah, and this issue is also present in a "standard" dired buffer.

So, apart from the missing newline at the end of the find-dired buffer,
I see no difference to regular dired buffers.

Michael.

bug#41624: 28.0.50; wdired-change-to-wdired-mode sometimes infloops

[Lars Ingebrigtsen]

Michael Heerdegen <michael_heerdegen@web.de> writes:

> I found out that the infloop can be fixed easily if we change
> `find-dired-sentinel' to let the buffer end in a newline.

[...]

>> Then I can change permissions with wdired, I only still get the
>> inappropriate message that it would have failed, though it has
>> succeeded.
>
> Hah, and this issue is also present in a "standard" dired buffer.
>
> So, apart from the missing newline at the end of the find-dired buffer,
> I see no difference to regular dired buffers.

Ah, then I think this should indeed be fixed (changing
`find-dired-sentinel' sounds good to me).  Are you pushing a fix for
this?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#41624: 28.0.50; wdired-change-to-wdired-mode sometimes infloops

[Michael Heerdegen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Ah, then I think this should indeed be fixed (changing
> `find-dired-sentinel' sounds good to me).  Are you pushing a fix for
> this?

Ok, will do.

In the meanwhile I have reported (and also partially debugged) the
"rename actions failed" issue as separate Bug #44343.

Regards,

Michael.