unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* [RFC] Do indent-region only on editable buffers
@ 2017-08-07 17:36 Kaushal Modi
  2017-08-07 17:46 ` Drew Adams
  2017-08-07 19:55 ` Stefan Monnier
  0 siblings, 2 replies; 7+ messages in thread
From: Kaushal Modi @ 2017-08-07 17:36 UTC (permalink / raw)
  To: Emacs developers

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

indent-region is an act of editing a buffer. So before attempting to do
that, it should be checked if the buffer is editable.

Bug#22819[1] raises that point.

The proposal is to call (barf-if-buffer-read-only) before attempting to do
the indent.

Motive for this change:

The act of indenting is an editing action. So the buffer should be checked
if it's editable before attempting an indent. If the buffer is read-only
and no indentation change is required, then good. But what if indentation
change is required? Here's what's will happen: 1. User: Try indentation
(C-x h M-x indent-region) 2. User: Could take several seconds or few
minutes (depending on major mode and file size) 3. Emacs: "Bummer, couldn't
save all that indentation because the buffer is read-only". 4. User: Make
buffer editable. It's not a simple act of chmod. In my case, the buffer was
read-only because the file is part of a centralized version control system
(Cliosoft SOS). In "checked in" state, the file is just a symlink to the
cached version in server, and thus read-only. To make it editable, I need
to "check out" the file. That act replaces the symlink link with a physical
file copy. 5. User: Re-do that several seconds/minutes long indentation.

So the motive is to save time and alert user of their pilot error before
attempting to do an operation that would go to waste (in this case, doing
indent, and then waste all those CPU cycles because the changes couldn't
get saved to the file).

Question to the list:

- Are there any objections to doing the buffer read-only check before doing
the indent?

(PS: If user's still want to do indentation on read-only buffers, then can
always do C-x C-q (toggle read-only-mode) and then proceed to indent the
buffer.)

[1]: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=22819
-- 

Kaushal Modi

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [RFC] Do indent-region only on editable buffers
  2017-08-07 17:36 [RFC] Do indent-region only on editable buffers Kaushal Modi
@ 2017-08-07 17:46 ` Drew Adams
  2017-08-07 17:56   ` Kaushal Modi
  2017-08-07 18:13   ` Noam Postavsky
  2017-08-07 19:55 ` Stefan Monnier
  1 sibling, 2 replies; 7+ messages in thread
From: Drew Adams @ 2017-08-07 17:46 UTC (permalink / raw)
  To: Kaushal Modi, Emacs developers

> - Are there any objections to doing the buffer read-only check before
>   doing the indent?

Dunno what the right approach/solution is in this particular case.

But note that we don't do what you suggest in general - we don't do it
for most other editing operations: insert or delete a character, yank
a string, kill the region,...  Instead, we let an error be raised,
telling the user that the buffer is read-only.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] Do indent-region only on editable buffers
  2017-08-07 17:46 ` Drew Adams
@ 2017-08-07 17:56   ` Kaushal Modi
  2017-08-07 18:13   ` Noam Postavsky
  1 sibling, 0 replies; 7+ messages in thread
From: Kaushal Modi @ 2017-08-07 17:56 UTC (permalink / raw)
  To: Drew Adams, Emacs developers

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

On Mon, Aug 7, 2017 at 1:47 PM Drew Adams <drew.adams@oracle.com> wrote:

> > - Are there any objections to doing the buffer read-only check before
> >   doing the indent?
>
> Dunno what the right approach/solution is in this particular case.
>
> But note that we don't do what you suggest in general - we don't do it
> for most other editing operations: insert or delete a character, yank
> a string, kill the region,...  Instead, we let an error be raised,
> telling the user that the buffer is read-only.
>

This case feels a bit different because the operation can be very
expensive.. a few seconds or couple of minutes long indentation, based on
the major mode and file size. So a user would find it tremendously helpful
to know that the operation is useless on a read-only buffer before spending
that amount of time.

With the cases of insert/delete char, yank string, kill region, I have
never noticed any tangible delay (read: even a second).. so in those, it do
not matter if you "do and then flag error" or "flag an error and then do".

Also there are many cases of commands meant for buffer editing (like
keep-lines, flush-lines, etc.) where barf-if-buffer-read-only is used for
the same purpose.

The purpose of this email thread is to understand if there's a workflow
someone uses that would break if this read-only check were done at the
beginning of the indentation operation. As Eli states his concern in the
Bug#22819, do people rely on side-effects other than the actual
buffer-editing indentation operation?
-- 

Kaushal Modi

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] Do indent-region only on editable buffers
  2017-08-07 17:46 ` Drew Adams
  2017-08-07 17:56   ` Kaushal Modi
@ 2017-08-07 18:13   ` Noam Postavsky
  1 sibling, 0 replies; 7+ messages in thread
From: Noam Postavsky @ 2017-08-07 18:13 UTC (permalink / raw)
  To: Drew Adams; +Cc: Emacs developers, Kaushal Modi

On Mon, Aug 7, 2017 at 1:46 PM, Drew Adams <drew.adams@oracle.com> wrote:
>> - Are there any objections to doing the buffer read-only check before
>>   doing the indent?
>
> Dunno what the right approach/solution is in this particular case.
>
> But note that we don't do what you suggest in general - we don't do it
> for most other editing operations: insert or delete a character, yank
> a string, kill the region,...  Instead, we let an error be raised,
> telling the user that the buffer is read-only.

The other commands you list finish pretty much instantaneously, so the
difference is somewhat academic there. Although yank has (interactive
"*P"), the star means the read-only check is at the beginning. So you
can't successfully yank the empty string into a read-only buffer.



^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] Do indent-region only on editable buffers
  2017-08-07 17:36 [RFC] Do indent-region only on editable buffers Kaushal Modi
  2017-08-07 17:46 ` Drew Adams
@ 2017-08-07 19:55 ` Stefan Monnier
  2017-08-07 20:07   ` Kaushal Modi
  2017-08-07 23:03   ` John Wiegley
  1 sibling, 2 replies; 7+ messages in thread
From: Stefan Monnier @ 2017-08-07 19:55 UTC (permalink / raw)
  To: emacs-devel

> The proposal is to call (barf-if-buffer-read-only) before attempting to do
> the indent.

No objection here.

> change is required? Here's what's will happen: 1. User: Try indentation
> (C-x h M-x indent-region) 2. User: Could take several seconds or few
> minutes (depending on major mode and file size) 3. Emacs: "Bummer, couldn't
> save all that indentation because the buffer is read-only". 4. User: Make

Indentation of a region isn't done by computing the complete result and
then "saving" it into the buffer.  So the error will be signaled as soon
as the indent-region finds a line whose indentation should be changed.
Usually that's almost instantaneous, but indeed in some cases it can
take a while (e.g. in large regions that are already indented except for
the last few lines).


        Stefan




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] Do indent-region only on editable buffers
  2017-08-07 19:55 ` Stefan Monnier
@ 2017-08-07 20:07   ` Kaushal Modi
  2017-08-07 23:03   ` John Wiegley
  1 sibling, 0 replies; 7+ messages in thread
From: Kaushal Modi @ 2017-08-07 20:07 UTC (permalink / raw)
  To: Stefan Monnier, emacs-devel

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

On Mon, Aug 7, 2017 at 3:55 PM Stefan Monnier <monnier@iro.umontreal.ca>
wrote:

> > The proposal is to call (barf-if-buffer-read-only) before attempting to
> do
> > the indent.
>
> No objection here.
>

Thanks.


> > change is required? Here's what's will happen: 1. User: Try indentation
> > (C-x h M-x indent-region) 2. User: Could take several seconds or few
> > minutes (depending on major mode and file size) 3. Emacs: "Bummer,
> couldn't
> > save all that indentation because the buffer is read-only". 4. User: Make
>
> Indentation of a region isn't done by computing the complete result and
> then "saving" it into the buffer.  So the error will be signaled as soon
> as the indent-region finds a line whose indentation should be changed.
> Usually that's almost instantaneous, but indeed in some cases it can
> take a while (e.g. in large regions that are already indented except for
> the last few lines).
>

Thanks for explaining the behavior. That explains why the read-only buffer
error is not thrown if the buffer is already indented.
-- 

Kaushal Modi

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

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [RFC] Do indent-region only on editable buffers
  2017-08-07 19:55 ` Stefan Monnier
  2017-08-07 20:07   ` Kaushal Modi
@ 2017-08-07 23:03   ` John Wiegley
  1 sibling, 0 replies; 7+ messages in thread
From: John Wiegley @ 2017-08-07 23:03 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: emacs-devel

>>>>> "SM" == Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> The proposal is to call (barf-if-buffer-read-only) before attempting to do
>> the indent.

SM> No objection here.

Nor here.

-- 
John Wiegley                  GPG fingerprint = 4710 CF98 AF9B 327B B80F
http://newartisans.com                          60E1 46C4 BD1A 7AC1 4BA2



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2017-08-07 23:03 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-08-07 17:36 [RFC] Do indent-region only on editable buffers Kaushal Modi
2017-08-07 17:46 ` Drew Adams
2017-08-07 17:56   ` Kaushal Modi
2017-08-07 18:13   ` Noam Postavsky
2017-08-07 19:55 ` Stefan Monnier
2017-08-07 20:07   ` Kaushal Modi
2017-08-07 23:03   ` John Wiegley

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).