unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#24766: 26.0.50: [PATCH] Confusing behaviour for indent-relative-maybe
@ 2016-10-22 19:01 Alex
  2016-10-22 19:21 ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Alex @ 2016-10-22 19:01 UTC (permalink / raw)
  To: 24766

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

In emacs -Q's scratch buffer, try the following:

M-: (indent-relative) RET

Repeating this will move to the next appropriate indentation point as
indicated in indent-relative's docstring.

Now try:

M-: (indent-relative-maybe) RET

The point does not move even when there are appropriate indentation
points to move to. This contradicts the intention of the docstring for
indent-relative-maybe:

       If the previous nonblank line has no indent points beyond the
       column point starts at, this command does nothing.


I would have expected, in indent-relative, that the calculation of a
suitable indentation position is done independent of the argument
UNINDENTED-OK. The following diff fixes this:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: indent diff --]
[-- Type: text/x-diff, Size: 1095 bytes --]

diff --git a/lisp/indent.el b/lisp/indent.el
index 0f6c68d..02ec210 100644
--- a/lisp/indent.el
+++ b/lisp/indent.el
@@ -593,18 +593,18 @@ indent-relative
 	    ;; Is start-column inside a tab on this line?
 	    (if (> (current-column) start-column)
 		(backward-char 1))
-	    (or (looking-at "[ \t]")
-		unindented-ok
-		(skip-chars-forward "^ \t" end))
+	    (unless (looking-at "[ \t]")
+              (skip-chars-forward "^ \t" end))
 	    (skip-chars-forward " \t" end)
 	    (or (= (point) end) (setq indent (current-column))))))
-    (if indent
-	(let ((opoint (point-marker)))
-	  (indent-to indent 0)
-	  (if (> opoint (point))
-	      (goto-char opoint))
-	  (move-marker opoint nil))
-      (tab-to-tab-stop))))
+    (cond (indent
+           (let ((opoint (point-marker)))
+             (indent-to indent 0)
+             (if (> opoint (point))
+                 (goto-char opoint))
+             (move-marker opoint nil)))
+          (unindented-ok nil)
+          (t (tab-to-tab-stop)))))
 
 (defcustom tab-stop-list nil
   "List of tab stop positions used by `tab-to-tab-stop'.

[-- Attachment #3: Type: text/plain, Size: 153 bytes --]



Apparently this stems from the initial revision of indent.el. Am I just
misinterpreting this function's purpose, or has it been wrong this whole
time?

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

* bug#24766: 26.0.50: [PATCH] Confusing behaviour for indent-relative-maybe
  2016-10-22 19:01 bug#24766: 26.0.50: [PATCH] Confusing behaviour for indent-relative-maybe Alex
@ 2016-10-22 19:21 ` Eli Zaretskii
  2016-10-22 19:40   ` Alex
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2016-10-22 19:21 UTC (permalink / raw)
  To: Alex; +Cc: 24766

> From: Alex <agrambot@gmail.com>
> Date: Sat, 22 Oct 2016 13:01:15 -0600
> 
> In emacs -Q's scratch buffer, try the following:
> 
> M-: (indent-relative) RET
> 
> Repeating this will move to the next appropriate indentation point as
> indicated in indent-relative's docstring.
> 
> Now try:
> 
> M-: (indent-relative-maybe) RET
> 
> The point does not move even when there are appropriate indentation
> points to move to.

It doesn't move because that's what UNINDENTED-OK means.

> This contradicts the intention of the docstring for
> indent-relative-maybe:
> 
>        If the previous nonblank line has no indent points beyond the
>        column point starts at, this command does nothing.
> 
> 
> I would have expected, in indent-relative, that the calculation of a
> suitable indentation position is done independent of the argument
> UNINDENTED-OK. The following diff fixes this:

These functions exist for ages in this form.  I agree that the doc
string is misleading (and the optional argument of indent-relative is
not even documented), but other than fixing the documentation, I see
no reason to change the behavior.  Am I missing something?





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

* bug#24766: 26.0.50: [PATCH] Confusing behaviour for indent-relative-maybe
  2016-10-22 19:21 ` Eli Zaretskii
@ 2016-10-22 19:40   ` Alex
  2016-10-22 19:48     ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Alex @ 2016-10-22 19:40 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 24766

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Date: Sat, 22 Oct 2016 13:01:15 -0600
>> 
>> In emacs -Q's scratch buffer, try the following:
>> 
>> M-: (indent-relative) RET
>> 
>> Repeating this will move to the next appropriate indentation point as
>> indicated in indent-relative's docstring.
>> 
>> Now try:
>> 
>> M-: (indent-relative-maybe) RET
>> 
>> The point does not move even when there are appropriate indentation
>> points to move to.
>
> It doesn't move because that's what UNINDENTED-OK means.

I took UNINTENDED-OK to mean that "if non-nil, nothing is done in the
case that there are no appropriate indentation positions. If there are
appropriate indentation positions, then it should indent as usual."

The docstring could be improved to state that.

>> This contradicts the intention of the docstring for
>> indent-relative-maybe:
>> 
>>        If the previous nonblank line has no indent points beyond the
>>        column point starts at, this command does nothing.
>> 
>> 
>> I would have expected, in indent-relative, that the calculation of a
>> suitable indentation position is done independent of the argument
>> UNINDENTED-OK. The following diff fixes this:
>
> These functions exist for ages in this form.  I agree that the doc
> string is misleading (and the optional argument of indent-relative is
> not even documented),

The optional argument is implicitly mentioned as "unless
this command is invoked with a numeric argument, in which case it
does nothing."

> but other than fixing the documentation, I see
> no reason to change the behavior.  Am I missing something?

IIUC the current behaviour essentially makes indent-relative-maybe a
no-op. But again, perhaps there's something I'm missing?





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

* bug#24766: 26.0.50: [PATCH] Confusing behaviour for indent-relative-maybe
  2016-10-22 19:40   ` Alex
@ 2016-10-22 19:48     ` Eli Zaretskii
  2016-10-22 21:49       ` Alex
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2016-10-22 19:48 UTC (permalink / raw)
  To: Alex; +Cc: 24766

> From: Alex <agrambot@gmail.com>
> Cc: 24766@debbugs.gnu.org
> Date: Sat, 22 Oct 2016 13:40:57 -0600
> 
> >> The point does not move even when there are appropriate indentation
> >> points to move to.
> >
> > It doesn't move because that's what UNINDENTED-OK means.
> 
> I took UNINTENDED-OK to mean that "if non-nil, nothing is done in the
> case that there are no appropriate indentation positions. If there are
> appropriate indentation positions, then it should indent as usual."
> 
> The docstring could be improved to state that.

Definitely.  Would you like to give it a try?

> The optional argument is implicitly mentioned as "unless
> this command is invoked with a numeric argument, in which case it
> does nothing."

Well, that's not how we document such functions, right?  Both the
effect of the argument, when used from Lisp, and the fact that it's
the prefix argument in interactive invocation, should be stated.

> > but other than fixing the documentation, I see
> > no reason to change the behavior.  Am I missing something?
> 
> IIUC the current behaviour essentially makes indent-relative-maybe a
> no-op.

No, it's definitely not a no-op.  It is only a no-op if the previous
non-blank line has no white space at its beginning, or the current
column is already past that first indentation point.  IOW,
indent-relative-maybe only ever indents to the first indentation
point, and only when that indentation point is preceded by whitespace.





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

* bug#24766: 26.0.50: [PATCH] Confusing behaviour for indent-relative-maybe
  2016-10-22 19:48     ` Eli Zaretskii
@ 2016-10-22 21:49       ` Alex
  2016-10-23  6:23         ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Alex @ 2016-10-22 21:49 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 24766

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: 24766@debbugs.gnu.org
>> Date: Sat, 22 Oct 2016 13:40:57 -0600
>> 
>> >> The point does not move even when there are appropriate indentation
>> >> points to move to.
>> >
>> > It doesn't move because that's what UNINDENTED-OK means.
>> 
>> I took UNINTENDED-OK to mean that "if non-nil, nothing is done in the
>> case that there are no appropriate indentation positions. If there are
>> appropriate indentation positions, then it should indent as usual."
>> 
>> The docstring could be improved to state that.
>
> Definitely.  Would you like to give it a try?

Sure. That would describe the behaviour that my diff brings -- does that
mean that you're okay with the proposed change?

>> The optional argument is implicitly mentioned as "unless
>> this command is invoked with a numeric argument, in which case it
>> does nothing."
>
> Well, that's not how we document such functions, right?  Both the
> effect of the argument, when used from Lisp, and the fact that it's
> the prefix argument in interactive invocation, should be stated.

Indeed. Though it seems, according to commit
1fd63d9b9bc249488ec12a49cc3a576baf8c788a, that you were the one to
document it. ;-)

>> > but other than fixing the documentation, I see
>> > no reason to change the behavior.  Am I missing something?
>> 
>> IIUC the current behaviour essentially makes indent-relative-maybe a
>> no-op.
>
> No, it's definitely not a no-op.  It is only a no-op if the previous
> non-blank line has no white space at its beginning, or the current
> column is already past that first indentation point.  IOW,
> indent-relative-maybe only ever indents to the first indentation
> point, and only when that indentation point is preceded by whitespace.

Oh, I see. Sorry about missing that. For some reason I was only testing
lines that started with non-whitespace.

I feel more hesitant to change such old behaviour now, but I still think
that it should be done. Here are the functions that call
indent-relative-maybe or call indent-relative with an argument:

* add-change-log-entry
* mh-letter-next-header-field-or-indent
* A few functions in AUCTeX (ELPA)

I'm not sure if they should be changed, but if they should, a new
function could be made to match the previous indent-relative-maybe
behaviour:

(let ((first-indent (save-excursion
                      (re-search-backward "^[^\n]")
                      (backward-to-indentation 0))))
  (when (< (current-column)
           first-indent)
    (indent-to first-indent)))






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

* bug#24766: 26.0.50: [PATCH] Confusing behaviour for indent-relative-maybe
  2016-10-22 21:49       ` Alex
@ 2016-10-23  6:23         ` Eli Zaretskii
  2016-10-23 20:43           ` Alex
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2016-10-23  6:23 UTC (permalink / raw)
  To: Alex; +Cc: 24766

> From: Alex <agrambot@gmail.com>
> Cc: 24766@debbugs.gnu.org
> Date: Sat, 22 Oct 2016 15:49:57 -0600
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> >> I took UNINTENDED-OK to mean that "if non-nil, nothing is done in the
> >> case that there are no appropriate indentation positions. If there are
> >> appropriate indentation positions, then it should indent as usual."
> >> 
> >> The docstring could be improved to state that.
> >
> > Definitely.  Would you like to give it a try?
> 
> Sure. That would describe the behaviour that my diff brings -- does that
> mean that you're okay with the proposed change?

No, I meant to improve the doc string to describe the current
behavior.

> >> The optional argument is implicitly mentioned as "unless
> >> this command is invoked with a numeric argument, in which case it
> >> does nothing."
> >
> > Well, that's not how we document such functions, right?  Both the
> > effect of the argument, when used from Lisp, and the fact that it's
> > the prefix argument in interactive invocation, should be stated.
> 
> Indeed. Though it seems, according to commit
> 1fd63d9b9bc249488ec12a49cc3a576baf8c788a, that you were the one to
> document it. ;-)

Yes, but my change just described one subtlety of that function, it
didn't change anything about the rest.  Most probably I bumped into
the aspect of the behavior I described, and found it not documented at
all.

> > No, it's definitely not a no-op.  It is only a no-op if the previous
> > non-blank line has no white space at its beginning, or the current
> > column is already past that first indentation point.  IOW,
> > indent-relative-maybe only ever indents to the first indentation
> > point, and only when that indentation point is preceded by whitespace.
> 
> Oh, I see. Sorry about missing that. For some reason I was only testing
> lines that started with non-whitespace.
> 
> I feel more hesitant to change such old behaviour now, but I still think
> that it should be done. Here are the functions that call
> indent-relative-maybe or call indent-relative with an argument:
> 
> * add-change-log-entry
> * mh-letter-next-header-field-or-indent
> * A few functions in AUCTeX (ELPA)
> 
> I'm not sure if they should be changed, but if they should, a new
> function could be made to match the previous indent-relative-maybe
> behaviour:
> 
> (let ((first-indent (save-excursion
>                       (re-search-backward "^[^\n]")
>                       (backward-to-indentation 0))))
>   (when (< (current-column)
>            first-indent)
>     (indent-to first-indent)))

Like I said, I don't think the behavior should be changed, only the
documentation, which is somewhat misleading.  If we want some
different behavior, we could have a new function, or a new value of
the argument to indent-relative.

Thanks.





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

* bug#24766: 26.0.50: [PATCH] Confusing behaviour for indent-relative-maybe
  2016-10-23  6:23         ` Eli Zaretskii
@ 2016-10-23 20:43           ` Alex
  2016-10-24  8:23             ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Alex @ 2016-10-23 20:43 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 24766

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

Eli Zaretskii <eliz@gnu.org> writes:

> Like I said, I don't think the behavior should be changed, only the
> documentation, which is somewhat misleading.  If we want some
> different behavior, we could have a new function, or a new value of
> the argument to indent-relative.
>
> Thanks.

What about changing `indent-relative-maybe' and adding an extra argument
to `indent-relative'? Since `indent-relative-maybe' has had that
docstring for a good 15 years, and the name fits the new behaviour more.

Then in Emacs core we can replace the single instance of it with
(indent-relative t). The corresponding diff:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: second time --]
[-- Type: text/x-diff, Size: 1430 bytes --]

diff --git a/lisp/indent.el b/lisp/indent.el
index 0f6c68d..675b6b8 100644
--- a/lisp/indent.el
+++ b/lisp/indent.el
@@ -566,9 +566,9 @@ indent-relative-maybe
 
 See also `indent-relative'."
   (interactive)
-  (indent-relative t))
+  (indent-relative nil t))
 
-(defun indent-relative (&optional unindented-ok)
+(defun indent-relative (&optional first-only unindented-ok)
   "Space out to under next indent point in previous nonblank line.
 An indent point is a non-whitespace character following whitespace.
 The following line shows the indentation points in this line.
@@ -594,17 +594,18 @@ indent-relative
 	    (if (> (current-column) start-column)
 		(backward-char 1))
 	    (or (looking-at "[ \t]")
-		unindented-ok
+		first-only
 		(skip-chars-forward "^ \t" end))
 	    (skip-chars-forward " \t" end)
 	    (or (= (point) end) (setq indent (current-column))))))
-    (if indent
-	(let ((opoint (point-marker)))
-	  (indent-to indent 0)
-	  (if (> opoint (point))
-	      (goto-char opoint))
-	  (move-marker opoint nil))
-      (tab-to-tab-stop))))
+    (cond (indent
+           (let ((opoint (point-marker)))
+             (indent-to indent 0)
+             (if (> opoint (point))
+                 (goto-char opoint))
+             (move-marker opoint nil)))
+          (unindented-ok nil)
+          (t (tab-to-tab-stop)))))
 
 (defcustom tab-stop-list nil
   "List of tab stop positions used by `tab-to-tab-stop'.

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

* bug#24766: 26.0.50: [PATCH] Confusing behaviour for indent-relative-maybe
  2016-10-23 20:43           ` Alex
@ 2016-10-24  8:23             ` Eli Zaretskii
  2016-10-24 19:27               ` Alex
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2016-10-24  8:23 UTC (permalink / raw)
  To: Alex; +Cc: 24766

> From: Alex <agrambot@gmail.com>
> Cc: 24766@debbugs.gnu.org
> Date: Sun, 23 Oct 2016 14:43:57 -0600
> 
> What about changing `indent-relative-maybe' and adding an extra argument
> to `indent-relative'? Since `indent-relative-maybe' has had that
> docstring for a good 15 years, and the name fits the new behaviour more.
> 
> Then in Emacs core we can replace the single instance of it with
> (indent-relative t). The corresponding diff:

I'd prefer a backward-compatible change, i.e. make the new argument be
the 2nd one, and keep the current behavior when the 1st arg is non-nil
and the 2nd is nil or omitted.

Also, please add the necessary text to the doc string to document the
new argument.

Thanks.





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

* bug#24766: 26.0.50: [PATCH] Confusing behaviour for indent-relative-maybe
  2016-10-24  8:23             ` Eli Zaretskii
@ 2016-10-24 19:27               ` Alex
  2016-10-24 19:57                 ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Alex @ 2016-10-24 19:27 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 24766

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: 24766@debbugs.gnu.org
>> Date: Sun, 23 Oct 2016 14:43:57 -0600
>> 
>> What about changing `indent-relative-maybe' and adding an extra argument
>> to `indent-relative'? Since `indent-relative-maybe' has had that
>> docstring for a good 15 years, and the name fits the new behaviour more.
>> 
>> Then in Emacs core we can replace the single instance of it with
>> (indent-relative t). The corresponding diff:
>
> I'd prefer a backward-compatible change, i.e. make the new argument be
> the 2nd one, and keep the current behavior when the 1st arg is non-nil
> and the 2nd is nil or omitted.

That's what I did, but I used a new name for the old argument and the
old name for the new argument. I did so as the old name fits the new
behaviour more.

This is a backward-compatible change for indent-relative, but it does
use the new behaviour for indent-relative-maybe. Is that alright with
you?

> Also, please add the necessary text to the doc string to document the
> new argument.

Sure, I'll do that once the code changes are agreed upon.

> Thanks.





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

* bug#24766: 26.0.50: [PATCH] Confusing behaviour for indent-relative-maybe
  2016-10-24 19:27               ` Alex
@ 2016-10-24 19:57                 ` Eli Zaretskii
  2016-11-08  1:53                   ` Alex
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2016-10-24 19:57 UTC (permalink / raw)
  To: Alex; +Cc: 24766

> From: Alex <agrambot@gmail.com>
> Cc: 24766@debbugs.gnu.org
> Date: Mon, 24 Oct 2016 13:27:43 -0600
> 
> > I'd prefer a backward-compatible change, i.e. make the new argument be
> > the 2nd one, and keep the current behavior when the 1st arg is non-nil
> > and the 2nd is nil or omitted.
> 
> That's what I did, but I used a new name for the old argument and the
> old name for the new argument. I did so as the old name fits the new
> behaviour more.
> 
> This is a backward-compatible change for indent-relative, but it does
> use the new behaviour for indent-relative-maybe. Is that alright with
> you?

Yes, thanks.





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

* bug#24766: 26.0.50: [PATCH] Confusing behaviour for indent-relative-maybe
  2016-10-24 19:57                 ` Eli Zaretskii
@ 2016-11-08  1:53                   ` Alex
  2016-11-08 15:07                     ` Eli Zaretskii
  0 siblings, 1 reply; 15+ messages in thread
From: Alex @ 2016-11-08  1:53 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 24766

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Alex <agrambot@gmail.com>
>> Cc: 24766@debbugs.gnu.org
>> Date: Mon, 24 Oct 2016 13:27:43 -0600
>> 
>> > I'd prefer a backward-compatible change, i.e. make the new argument be
>> > the 2nd one, and keep the current behavior when the 1st arg is non-nil
>> > and the 2nd is nil or omitted.
>> 
>> That's what I did, but I used a new name for the old argument and the
>> old name for the new argument. I did so as the old name fits the new
>> behaviour more.
>> 
>> This is a backward-compatible change for indent-relative, but it does
>> use the new behaviour for indent-relative-maybe. Is that alright with
>> you?
>
> Yes, thanks.

Sorry for the delay.

After thinking about it some more, and after properly searching on
Github for `indent-relative-maybe', I'm not sure if my previous solution
is the best one now. I found that due to some blog posts and starter kit
configurations, a surprising amount of people use indent-relative-maybe
despite docstring claiming different functionality.

I now think the following should happen:

1) indent-relative-maybe's should be obsoleted in
favour of a name suiting the purpose (e.g. indent-relative-whitespace)
with a better docstring.

2) The docstring and second optional argument should be added as
discussed before.

3) Perhaps in the future a new function can be introduced that
automatically calls (indent-relative nil t), but I'm not sure if that
should be done now. To be honest, I lost my original reason that made me
interested in this function.

Anyway, I've attached a diff that addresses this new proposal.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 3rd time --]
[-- Type: text/x-diff, Size: 2441 bytes --]

diff --git a/lisp/indent.el b/lisp/indent.el
index 0f6c68d..6c907f4 100644
--- a/lisp/indent.el
+++ b/lisp/indent.el
@@ -559,24 +559,29 @@ indent-region
   ;; by hand.
   (setq deactivate-mark t))
 
-(defun indent-relative-maybe ()
-  "Indent a new line like previous nonblank line.
-If the previous nonblank line has no indent points beyond the
-column point starts at, this command does nothing.
+(define-obsolete-function-alias 'indent-relative-maybe
+  'indent-relative-whitespace "26.1")
+
+(defun indent-relative-whitespace ()
+  "Indent the current line like the previous nonblank line.
+Indent to the first indentation position in the previous nonblank
+line.
 
 See also `indent-relative'."
   (interactive)
   (indent-relative t))
 
-(defun indent-relative (&optional unindented-ok)
+(defun indent-relative (&optional first-only unindented-ok)
   "Space out to under next indent point in previous nonblank line.
 An indent point is a non-whitespace character following whitespace.
 The following line shows the indentation points in this line.
     ^         ^    ^     ^   ^           ^      ^  ^    ^
+If FIRST-ONLY is non-nil, then only the first indent point is
+considered.
+
 If the previous nonblank line has no indent points beyond the
-column point starts at, `tab-to-tab-stop' is done instead, unless
-this command is invoked with a numeric argument, in which case it
-does nothing.
+column point starts at, then `tab-to-tab-stop' is done if
+UNINDENTED-OK is nil, otherwise nothing is done in this case.
 
 See also `indent-relative-maybe'."
   (interactive "P")
@@ -594,17 +599,18 @@ indent-relative
 	    (if (> (current-column) start-column)
 		(backward-char 1))
 	    (or (looking-at "[ \t]")
-		unindented-ok
+		first-only
 		(skip-chars-forward "^ \t" end))
 	    (skip-chars-forward " \t" end)
 	    (or (= (point) end) (setq indent (current-column))))))
-    (if indent
-	(let ((opoint (point-marker)))
-	  (indent-to indent 0)
-	  (if (> opoint (point))
-	      (goto-char opoint))
-	  (move-marker opoint nil))
-      (tab-to-tab-stop))))
+    (cond (indent
+           (let ((opoint (point-marker)))
+             (indent-to indent 0)
+             (if (> opoint (point))
+                 (goto-char opoint))
+             (move-marker opoint nil)))
+          (unindented-ok nil)
+          (t (tab-to-tab-stop)))))
 
 (defcustom tab-stop-list nil
   "List of tab stop positions used by `tab-to-tab-stop'.

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

* bug#24766: 26.0.50: [PATCH] Confusing behaviour for indent-relative-maybe
  2016-11-08  1:53                   ` Alex
@ 2016-11-08 15:07                     ` Eli Zaretskii
  2016-11-08 19:09                       ` Alex
  0 siblings, 1 reply; 15+ messages in thread
From: Eli Zaretskii @ 2016-11-08 15:07 UTC (permalink / raw)
  To: Alex; +Cc: 24766

> From: Alex <agrambot@gmail.com>
> Cc: 24766@debbugs.gnu.org
> Date: Mon, 07 Nov 2016 19:53:22 -0600
> 
> After thinking about it some more, and after properly searching on
> Github for `indent-relative-maybe', I'm not sure if my previous solution
> is the best one now. I found that due to some blog posts and starter kit
> configurations, a surprising amount of people use indent-relative-maybe
> despite docstring claiming different functionality.
> 
> I now think the following should happen:
> 
> 1) indent-relative-maybe's should be obsoleted in
> favour of a name suiting the purpose (e.g. indent-relative-whitespace)
> with a better docstring.
> 
> 2) The docstring and second optional argument should be added as
> discussed before.
> 
> 3) Perhaps in the future a new function can be introduced that
> automatically calls (indent-relative nil t), but I'm not sure if that
> should be done now. To be honest, I lost my original reason that made me
> interested in this function.
> 
> Anyway, I've attached a diff that addresses this new proposal.

Thanks, this is fine, except that the new function's name should be
better as indent-relative-first-indent-point or somesuch.





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

* bug#24766: 26.0.50: [PATCH] Confusing behaviour for indent-relative-maybe
  2016-11-08 15:07                     ` Eli Zaretskii
@ 2016-11-08 19:09                       ` Alex
  2016-11-08 20:26                         ` Eli Zaretskii
  2016-11-18  9:04                         ` Eli Zaretskii
  0 siblings, 2 replies; 15+ messages in thread
From: Alex @ 2016-11-08 19:09 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 24766

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

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, this is fine, except that the new function's name should be
> better as indent-relative-first-indent-point or somesuch.

That name does better describe how (indent-relative t) works, but I
figured that indent-relative-whitespace is a better name for what people
seem to use it for (indent according to the whitespace on the previous
nonblank line).

Though if you still disagree then `indent-relative-first-indent-point'
seems fine (if not a bit long).

I suppose this diff goes over the accepted limit for non-assigned
submissions? I mailed my copyright forms about a month ago but
unfortunately the FSF hasn't informed me that the process is complete
yet.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 4th time --]
[-- Type: text/x-diff, Size: 2649 bytes --]

diff --git a/lisp/indent.el b/lisp/indent.el
index 0f6c68d..6d04192 100644
--- a/lisp/indent.el
+++ b/lisp/indent.el
@@ -559,26 +559,32 @@ indent-region
   ;; by hand.
   (setq deactivate-mark t))
 
-(defun indent-relative-maybe ()
-  "Indent a new line like previous nonblank line.
-If the previous nonblank line has no indent points beyond the
-column point starts at, this command does nothing.
+(define-obsolete-function-alias 'indent-relative-maybe
+  'indent-relative-first-indent-point "26.1")
+
+(defun indent-relative-first-indent-point ()
+  "Indent the current line like the previous nonblank line.
+Indent to the first indentation position in the previous nonblank
+line if that position is greater than the current column.
 
 See also `indent-relative'."
   (interactive)
   (indent-relative t))
 
-(defun indent-relative (&optional unindented-ok)
+(defun indent-relative (&optional first-only unindented-ok)
   "Space out to under next indent point in previous nonblank line.
 An indent point is a non-whitespace character following whitespace.
 The following line shows the indentation points in this line.
     ^         ^    ^     ^   ^           ^      ^  ^    ^
+If FIRST-ONLY is non-nil, then only the first indent point is
+considered.
+
 If the previous nonblank line has no indent points beyond the
-column point starts at, `tab-to-tab-stop' is done instead, unless
-this command is invoked with a numeric argument, in which case it
-does nothing.
+column point starts at, then `tab-to-tab-stop' is done if both
+FIRST-ONLY and UNINDENTED-OK are nil, otherwise nothing is done
+in this case.
 
-See also `indent-relative-maybe'."
+See also `indent-relative-first-indent-point'."
   (interactive "P")
   (if (and abbrev-mode
 	   (eq (char-syntax (preceding-char)) ?w))
@@ -594,17 +600,18 @@ indent-relative
 	    (if (> (current-column) start-column)
 		(backward-char 1))
 	    (or (looking-at "[ \t]")
-		unindented-ok
+		first-only
 		(skip-chars-forward "^ \t" end))
 	    (skip-chars-forward " \t" end)
 	    (or (= (point) end) (setq indent (current-column))))))
-    (if indent
-	(let ((opoint (point-marker)))
-	  (indent-to indent 0)
-	  (if (> opoint (point))
-	      (goto-char opoint))
-	  (move-marker opoint nil))
-      (tab-to-tab-stop))))
+    (cond (indent
+           (let ((opoint (point-marker)))
+             (indent-to indent 0)
+             (if (> opoint (point))
+                 (goto-char opoint))
+             (move-marker opoint nil)))
+          (unindented-ok nil)
+          (t (tab-to-tab-stop)))))
 
 (defcustom tab-stop-list nil
   "List of tab stop positions used by `tab-to-tab-stop'.

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

* bug#24766: 26.0.50: [PATCH] Confusing behaviour for indent-relative-maybe
  2016-11-08 19:09                       ` Alex
@ 2016-11-08 20:26                         ` Eli Zaretskii
  2016-11-18  9:04                         ` Eli Zaretskii
  1 sibling, 0 replies; 15+ messages in thread
From: Eli Zaretskii @ 2016-11-08 20:26 UTC (permalink / raw)
  To: Alex; +Cc: 24766

> From: Alex <agrambot@gmail.com>
> Cc: 24766@debbugs.gnu.org
> Date: Tue, 08 Nov 2016 13:09:42 -0600
> 
> I suppose this diff goes over the accepted limit for non-assigned
> submissions? I mailed my copyright forms about a month ago but
> unfortunately the FSF hasn't informed me that the process is complete
> yet.

Please ping them.





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

* bug#24766: 26.0.50: [PATCH] Confusing behaviour for indent-relative-maybe
  2016-11-08 19:09                       ` Alex
  2016-11-08 20:26                         ` Eli Zaretskii
@ 2016-11-18  9:04                         ` Eli Zaretskii
  1 sibling, 0 replies; 15+ messages in thread
From: Eli Zaretskii @ 2016-11-18  9:04 UTC (permalink / raw)
  To: Alex; +Cc: 24766

> From: Alex <agrambot@gmail.com>
> Cc: 24766@debbugs.gnu.org
> Date: Tue, 08 Nov 2016 13:09:42 -0600
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Thanks, this is fine, except that the new function's name should be
> > better as indent-relative-first-indent-point or somesuch.
> 
> That name does better describe how (indent-relative t) works, but I
> figured that indent-relative-whitespace is a better name for what people
> seem to use it for (indent according to the whitespace on the previous
> nonblank line).

Thanks, pushed to the master branch.





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

end of thread, other threads:[~2016-11-18  9:04 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-22 19:01 bug#24766: 26.0.50: [PATCH] Confusing behaviour for indent-relative-maybe Alex
2016-10-22 19:21 ` Eli Zaretskii
2016-10-22 19:40   ` Alex
2016-10-22 19:48     ` Eli Zaretskii
2016-10-22 21:49       ` Alex
2016-10-23  6:23         ` Eli Zaretskii
2016-10-23 20:43           ` Alex
2016-10-24  8:23             ` Eli Zaretskii
2016-10-24 19:27               ` Alex
2016-10-24 19:57                 ` Eli Zaretskii
2016-11-08  1:53                   ` Alex
2016-11-08 15:07                     ` Eli Zaretskii
2016-11-08 19:09                       ` Alex
2016-11-08 20:26                         ` Eli Zaretskii
2016-11-18  9:04                         ` Eli Zaretskii

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).