Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

classic Classic list List threaded Threaded
179 messages Options
1234 ... 9
Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Stefan Monnier
>     Replace prog-widen with consolidating widen calls

So, IIUC the idea is that the multi-major-mode framework will *have* to
use narrowing to indicate to the sub-modes the boundaries of the current
chunk, right?


        Stefan


> ---
>  doc/lispref/text.texi       | 45 +++++----------------------------------------
>  etc/NEWS                    |  8 ++++----
>  lisp/indent.el              | 12 +++++++++---
>  lisp/progmodes/prog-mode.el | 28 ++--------------------------
>  lisp/progmodes/python.el    | 34 ++++++++--------------------------
>  lisp/progmodes/ruby-mode.el |  1 -
>  lisp/vc/add-log.el          |  4 +++-
>  7 files changed, 31 insertions(+), 101 deletions(-)

> diff --git a/doc/lispref/text.texi b/doc/lispref/text.texi
> index 1e19f75..5a63e5b 100644
> --- a/doc/lispref/text.texi
> +++ b/doc/lispref/text.texi
> @@ -2407,42 +2407,17 @@ such multi-mode indentation.
>  This variable, when non-@code{nil}, holds the indentation context for
>  the sub-mode's indentation engine provided by the superior major mode.
>  The value should be a list of the form @code{(@var{first-column}
> -@w{(@var{start} . @var{end})} @code{prev-chunk})}.  The members of the
> -list have the following meaning:
> +@code{...rest})}.  The members of the list have the following meaning:
 

>  @table @var
>  @item first-column
>  The column to be used for top-level constructs.  This replaces the
>  default value of the top-level column used by the sub-mode, usually
>  zero.
> -@item start
> -@itemx end
> -The region of the code chunk to be indented by the sub-mode.  The
> -value of @var{end} can be @code{nil}, which stands for the value of
> -@code{point-max}.
> -@item prev-chunk
> -If this is non-@code{nil}, it should provide the sub-mode's
> -indentation engine with a virtual context of the code chunk.  Valid
> -values include:
> -
> -@itemize @minus
> -@item
> -A string whose contents is the text the sub-mode's indentation engine
> -should consider to precede the code chunk.  The sub-mode's indentation
> -engine can add text properties to that string, to be reused in
> -repeated calls with the same string, thus using it as a cache.  An
> -example where this is useful is code chunks that need to be indented
> -as function bodies, but lack the function's preamble---the string
> -could then include that missing preamble.
> -@item
> -A function.  It is expected to be called with the start position of
> -the current chunk, and should return a cons cell
> -@w{@code{(@var{prev-start} . @var{prev-end})}} that specifies the
> -region of the previous code chunk, or @code{nil} if there is no previous
> -chunk.  This is useful in literate-programming sources, where code is
> -split into chunks, and correct indentation needs to access previous
> -chunks.
> -@end itemize
> +@item rest
> +Reserved for future use.  The intent is to provide the sub-mode's
> +indentation engine with a virtual context of the code chunk.  Working
> +patches that use this in a significant fashion are welcome.
>  @end table
>  @end defvar
 
> @@ -2457,16 +2432,6 @@ function's value is the column number to use for top-level constructs.
>  When no superior mode is in effect, this function returns zero.
>  @end defun
 

> -@defun prog-widen
> -Call this function instead of @code{widen} to remove any restrictions
> -imposed by the mode's indentation engine and restore the restrictions
> -recorded in @code{prog-indentation-context}.  This prevents the
> -indentation engine of a sub-mode from inadvertently operating on text
> -outside of the chunk it was supposed to indent, and preserves the
> -restriction imposed by the superior mode.  When no superior mode is in
> -effect, this function just calls @code{widen}.
> -@end defun
> -
 
>  @node Region Indent
>  @subsection Indenting an Entire Region
> diff --git a/etc/NEWS b/etc/NEWS
> index 4ccf468..6e1561d 100644
> --- a/etc/NEWS
> +++ b/etc/NEWS
> @@ -1108,10 +1108,10 @@ programming languages in the same buffer, like literate programming
>  environments or ANTLR programs with embedded Python code.
 
>  A major mode can provide indentation context for a sub-mode through
> -the 'prog-indentation-context' variable.  To support this, modes that
> -provide indentation should use 'prog-widen' instead of 'widen' and
> -'prog-first-column' instead of a literal zero.  See the node
> -"(elisp) Mode-Specific Indent" in the ELisp manual for more details.
> +the 'prog-indentation-context' variable.  To support this, modes
> +should use 'prog-first-column' instead of a literal zero and never
> +call 'widen' in their indentation functions.  See the node "(elisp)
> +Mode-Specific Indent" in the ELisp manual for more details.
 
>  ** ERC
 

> diff --git a/lisp/indent.el b/lisp/indent.el
> index d5ba0bd..ccf0e99 100644
> --- a/lisp/indent.el
> +++ b/lisp/indent.el
> @@ -69,6 +69,8 @@ variable is `indent-relative' or `indent-relative-maybe', handle
>  it specially (since those functions are used for tabbing); in
>  that case, indent by aligning to the previous non-blank line."
>    (interactive)
> +  (save-restriction
> +    (widen)
>    (syntax-propertize (line-end-position))
>    (if (memq indent-line-function
>      '(indent-relative indent-relative-maybe))
> @@ -84,7 +86,7 @@ that case, indent by aligning to the previous non-blank line."
>      (indent-line-to column)
>    (save-excursion (indent-line-to column))))
>      ;; The normal case.
> -    (funcall indent-line-function)))
> +    (funcall indent-line-function))))
 

>  (defun indent--default-inside-comment ()
>    (unless (or (> (current-column) (current-indentation))
> @@ -144,7 +146,9 @@ prefix argument is ignored."
>            (indent--default-inside-comment)
>            (when (or (<= (current-column) (current-indentation))
>                      (not (eq tab-always-indent 'complete)))
> -            (funcall (default-value 'indent-line-function))))
> +            (save-restriction
> +              (widen)
> +              (funcall (default-value 'indent-line-function)))))
 

>        (cond
>         ;; If the text was already indented right, try completion.
> @@ -538,7 +542,9 @@ column to indent to; if it is nil, use one of the three methods above."
>    (forward-line 1)))))
>     ;; Use indent-region-function is available.
>     (indent-region-function
> -    (funcall indent-region-function start end))
> +    (save-restriction
> +      (widen)
> +      (funcall indent-region-function start end)))
>     ;; Else, use a default implementation that calls indent-line-function on
>     ;; each line.
>     (t (indent-region-line-by-line start end)))
> diff --git a/lisp/progmodes/prog-mode.el b/lisp/progmodes/prog-mode.el
> index f727e45..b36d242 100644
> --- a/lisp/progmodes/prog-mode.el
> +++ b/lisp/progmodes/prog-mode.el
> @@ -64,37 +64,13 @@ mode, it should bind this variable to non-nil around the call.
 
>  The non-nil value should be a list of the form:
 
> -   (FIRST-COLUMN (START . END) PREVIOUS-CHUNKS)
> +   (FIRST-COLUMN ...REST)
 
>  FIRST-COLUMN is the column the indentation engine of the sub-mode
>  should use for top-level language constructs inside the code
>  chunk (instead of 0).
 

> -START and END specify the region of the code chunk.  END can be
> -nil, which stands for the value of `point-max'.  The function
> -`prog-widen' uses this to restore restrictions imposed by the
> -sub-mode's indentation engine.
> -
> -PREVIOUS-CHUNKS, if non-nil, provides the indentation engine of
> -the sub-mode with the virtual context of the code chunk.  Valid
> -values are:
> -
> - - A string containing text which the indentation engine can
> -   consider as standing in front of the code chunk.  To cache the
> -   string's calculated syntactic information for repeated calls
> -   with the same string, the sub-mode can add text-properties to
> -   the string.
> -
> -   A typical use case is for grammars with code chunks which are
> -   to be indented like function bodies -- the string would contain
> -   the corresponding function preamble.
> -
> - - A function, to be called with the start position of the current
> -   chunk.  It should return either the region of the previous chunk
> -   as (PREV-START . PREV-END), or nil if there is no previous chunk.
> -
> -   A typical use case are literate programming sources -- the
> -   function would successively return the previous code chunks.")
> +REST is currently unused, but can be defined in future versions.")
 
>  (defun prog-indent-sexp (&optional defun)
>    "Indent the expression after point.
> diff --git a/lisp/progmodes/python.el b/lisp/progmodes/python.el
> index d4226e5..104889a 100644
> --- a/lisp/progmodes/python.el
> +++ b/lisp/progmodes/python.el
> @@ -287,10 +287,6 @@
>  ;;; 24.x Compat
>  
 

> -(unless (fboundp 'prog-widen)
> -  (defun prog-widen ()
> -    (widen)))
> -
>  (unless (fboundp 'prog-first-column)
>    (defun prog-first-column ()
>      0))
> @@ -785,7 +781,7 @@ work on `python-indent-calculate-indentation' instead."
>    (interactive)
>    (save-excursion
>      (save-restriction
> -      (prog-widen)
> +      (widen)
>        (goto-char (point-min))
>        (let ((block-end))
>          (while (and (not block-end)
> @@ -883,8 +879,6 @@ keyword
>  :at-dedenter-block-start
>   - Point is on a line starting a dedenter block.
>   - START is the position where the dedenter block starts."
> -  (save-restriction
> -    (prog-widen)
>      (let ((ppss (save-excursion
>                    (beginning-of-line)
>                    (syntax-ppss))))
> @@ -1022,7 +1016,7 @@ keyword
>                        (looking-at (python-rx block-ender)))
>                      :after-block-end)
>                     (t :after-line))
> -             (point)))))))))
> +             (point))))))))
 

>  (defun python-indent--calculate-indentation ()
>    "Internal implementation of `python-indent-calculate-indentation'.
> @@ -1030,8 +1024,6 @@ May return an integer for the maximum possible indentation at
>  current context or a list of integers.  The latter case is only
>  happening for :at-dedenter-block-start context since the
>  possibilities can be narrowed to specific indentation points."
> -  (save-restriction
> -    (prog-widen)
>      (save-excursion
>        (pcase (python-indent-context)
>          (`(:no-indent . ,_) (prog-first-column)) ; usually 0
> @@ -1081,7 +1073,7 @@ possibilities can be narrowed to specific indentation points."
>          (`(,(or :inside-paren-newline-start-from-block) . ,start)
>           ;; Add two indentation levels to make the suite stand out.
>           (goto-char start)
> -         (+ (current-indentation) (* python-indent-offset 2)))))))
> +         (+ (current-indentation) (* python-indent-offset 2))))))
 

>  (defun python-indent--calculate-levels (indentation)
>    "Calculate levels list given INDENTATION.
> @@ -4589,8 +4581,6 @@ To this:
>  Optional argument INCLUDE-TYPE indicates to include the type of the defun.
>  This function can be used as the value of `add-log-current-defun-function'
>  since it returns nil if point is not inside a defun."
> -  (save-restriction
> -    (prog-widen)
>      (save-excursion
>        (end-of-line 1)
>        (let ((names)
> @@ -4644,7 +4634,7 @@ since it returns nil if point is not inside a defun."
>              (and (= (current-indentation) 0) (throw 'exit t))))
>          (and names
>               (concat (and type (format "%s " type))
> -                     (mapconcat 'identity names ".")))))))
> +                     (mapconcat 'identity names "."))))))
 

>  (defun python-info-current-symbol (&optional replace-self)
>    "Return current symbol using dotty syntax.
> @@ -4788,12 +4778,10 @@ likely an invalid python file."
>    "Message the first line of the block the current statement closes."
>    (let ((point (python-info-dedenter-opening-block-position)))
>      (when point
> -      (save-restriction
> -        (prog-widen)
>          (message "Closes %s" (save-excursion
>                                 (goto-char point)
>                                 (buffer-substring
> -                                (point) (line-end-position))))))))
> +                                (point) (line-end-position)))))))
 

>  (defun python-info-dedenter-statement-p ()
>    "Return point if current statement is a dedenter.
> @@ -4809,8 +4797,6 @@ statement."
>    "Return non-nil if current line ends with backslash.
>  With optional argument LINE-NUMBER, check that line instead."
>    (save-excursion
> -    (save-restriction
> -      (prog-widen)
>        (when line-number
>          (python-util-goto-line line-number))
>        (while (and (not (eobp))
> @@ -4819,14 +4805,12 @@ With optional argument LINE-NUMBER, check that line instead."
>                    (not (equal (char-before (point)) ?\\)))
>          (forward-line 1))
>        (when (equal (char-before) ?\\)
> -        (point-marker)))))
> +        (point-marker))))
 

>  (defun python-info-beginning-of-backslash (&optional line-number)
>    "Return the point where the backslashed line start.
>  Optional argument LINE-NUMBER forces the line number to check against."
>    (save-excursion
> -    (save-restriction
> -      (prog-widen)
>        (when line-number
>          (python-util-goto-line line-number))
>        (when (python-info-line-ends-backslash-p)
> @@ -4835,15 +4819,13 @@ Optional argument LINE-NUMBER forces the line number to check against."
>                   (python-syntax-context 'paren))
>            (forward-line -1))
>          (back-to-indentation)
> -        (point-marker)))))
> +        (point-marker))))
 

>  (defun python-info-continuation-line-p ()
>    "Check if current line is continuation of another.
>  When current line is continuation of another return the point
>  where the continued line ends."
>    (save-excursion
> -    (save-restriction
> -      (prog-widen)
>        (let* ((context-type (progn
>                               (back-to-indentation)
>                               (python-syntax-context-type)))
> @@ -4869,7 +4851,7 @@ where the continued line ends."
>                 (python-util-forward-comment -1)
>                 (when (and (equal (1- line-start) (line-number-at-pos))
>                            (python-info-line-ends-backslash-p))
> -                 (point-marker))))))))
> +                 (point-marker)))))))
 

>  (defun python-info-block-continuation-line-p ()
>    "Return non-nil if current line is a continuation of a block."
> diff --git a/lisp/progmodes/ruby-mode.el b/lisp/progmodes/ruby-mode.el
> index dc1b0f8..7c10114 100644
> --- a/lisp/progmodes/ruby-mode.el
> +++ b/lisp/progmodes/ruby-mode.el
> @@ -1364,7 +1364,6 @@ delimiter."
>                                               "\\)\\>")))
>                      (eq (ruby-deep-indent-paren-p t) 'space)
>                      (not (bobp)))
> -                   (widen)
>                     (goto-char (or begin parse-start))
>                     (skip-syntax-forward " ")
>                     (current-column))
> diff --git a/lisp/vc/add-log.el b/lisp/vc/add-log.el
> index 392147b..9da8abc 100644
> --- a/lisp/vc/add-log.el
> +++ b/lisp/vc/add-log.el
> @@ -1133,6 +1133,8 @@ identifiers followed by `:' or `='.  See variables
>  Has a preference of looking backwards."
>    (condition-case nil
>        (save-excursion
> +        (save-restriction
> +          (widen)
>   (if add-log-current-defun-function
>      (funcall add-log-current-defun-function)
>    ;; If all else fails, try heuristics
> @@ -1147,7 +1149,7 @@ Has a preference of looking backwards."
>        (when (string-match "\\([^ \t\n\r\f].*[^ \t\n\r\f]\\)"
>    result)
>   (setq result (match-string-no-properties 1 result)))
> -      result))))
> +      result)))))
>      (error nil)))
 
>  (defvar change-log-get-method-definition-md)

> _______________________________________________
> Emacs-diffs mailing list
> [hidden email]
> https://lists.gnu.org/mailman/listinfo/emacs-diffs

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Dmitry Gutov
On 11/30/17 1:53 AM, Stefan Monnier wrote:
>>      Replace prog-widen with consolidating widen calls
>
> So, IIUC the idea is that the multi-major-mode framework will *have* to
> use narrowing to indicate to the sub-modes the boundaries of the current
> chunk, right?

Yes. Just like it would *have* to bind prog-indentation-context to a
complex-ish value in the previous proposal.

I can't imagine a case where it would be inconvenient to do so.

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Dmitry Gutov
On 11/30/17 8:59 AM, Dmitry Gutov wrote:
> On 11/30/17 1:53 AM, Stefan Monnier wrote:
>>>      Replace prog-widen with consolidating widen calls
>>
>> So, IIUC the idea is that the multi-major-mode framework will *have* to
>> use narrowing to indicate to the sub-modes the boundaries of the current
>> chunk, right?
>
> Yes. Just like it would *have* to bind prog-indentation-context to a
> complex-ish value in the previous proposal.

And while the difference isn't night and day, allow me to elaborate why
I think this is a move in a good direction:

- Major mode authors have been forgetting to widen in all the cases they
actually have to. Or they would do that in indent-line-function, but
forget in beginning-of-defun-function, etc. Now, they are *mandated* to
obey the current narrowing (deleting code is easy, and some won't have
delete anything at all). The indentation code might fail to work inside
narrowing (like all of CC Mode, probably), but that's an orthogonal issue.

- Multi-mode packages have been using narrowing for this purpose for
years, so they won't have to change much.

And, it's kind of funny, mhtml-indent-line which we've reviewed and
added recently, uses narrow-to-region too. :-) While disregarding the
second element of prog-indentation-context (which it set to the value
"widen all you want"). Which works well enough because SMIE doesn't
widen, and I've removed the `widen' call from js-mode indentation code
3.5 years ago.

I'm adding Tom and Vitalie to Cc. Maybe they have something to add.

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Tom Tromey-4
>>>>> "Dmitry" == Dmitry Gutov <[hidden email]> writes:

Dmitry> And, it's kind of funny, mhtml-indent-line which we've reviewed and
Dmitry> added recently, uses narrow-to-region too. :-) While disregarding the
Dmitry> second element of prog-indentation-context (which it set to the value
Dmitry> "widen all you want"). Which works well enough because SMIE doesn't
Dmitry> widen, and I've removed the `widen' call from js-mode indentation code
Dmitry> 3.5 years ago.

Yes, IIRC mhtml-mode narrows so that a sub-mode won't try to examine
parts of the buffer that have a different syntax.  Whether you consider
this ugly or not is perhaps related to your stance on the whole widening
issue.  My view, though, is that there is a problem here intrinsic to
multi-modes: something has to tell the sub-modes about the relevant
boundaries in some way; so if the narrowing/widening stuff is changed,
something new will have to be invented here.

Tom

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Dmitry Gutov
On 11/30/17 3:31 PM, Tom Tromey wrote:
> Yes, IIRC mhtml-mode narrows so that a sub-mode won't try to examine
> parts of the buffer that have a different syntax.  Whether you consider
> this ugly or not is perhaps related to your stance on the whole widening
> issue.  My view, though, is that there is a problem here intrinsic to
> multi-modes: something has to tell the sub-modes about the relevant
> boundaries in some way;

Err, no. Ugly or not is beside the point (I personally prefer this
approach, please also take a look at the branch referenced in this
thread subject).

Thing is, if you're using prog-indentation-context as it's now
implemented, you're "supposed to" indicate the narrowing bounds via
setting the second element of that list, instead of calling
narrow-to-region directly. Of course, that also requires support from
the major modes in question.

 > so if the narrowing/widening stuff is changed,
 > something new will have to be invented here.

See above. The "something new" has a few rough edges, so this discussion
is about revisiting those choices.

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Alan Mackenzie
In reply to this post by Dmitry Gutov
Hello, Dmitry.

On Thu, Nov 30, 2017 at 10:58:47 +0000, Dmitry Gutov wrote:
> On 11/30/17 8:59 AM, Dmitry Gutov wrote:
> > On 11/30/17 1:53 AM, Stefan Monnier wrote:
> >>>      Replace prog-widen with consolidating widen calls

> >> So, IIUC the idea is that the multi-major-mode framework will *have* to
> >> use narrowing to indicate to the sub-modes the boundaries of the current
> >> chunk, right?

Using narrowing for marking the bounds of a sub-mode is a bad thing,
since it is likely to cause contention with other uses of narrowing.
There are better ways.

> > Yes. Just like it would *have* to bind prog-indentation-context to a
> > complex-ish value in the previous proposal.

> And while the difference isn't night and day, allow me to elaborate why
> I think this is a move in a good direction:

> - Major mode authors have been forgetting to widen in all the cases they
> actually have to. Or they would do that in indent-line-function, but
> forget in beginning-of-defun-function, etc. Now, they are *mandated* to
> obey the current narrowing

It's not clear what is meant here, but mandating maintainers of major
modes to use narrowing in a particular way is at best controversial, and
probably will render many major modes non-functional.

> (deleting code is easy, and some won't have delete anything at all).
> The indentation code might fail to work inside narrowing (like all of
> CC Mode, probably), but that's an orthogonal issue.

Narrowing belongs to users and major modes.  I am lacking the context of
your post, which appears to start deep down in some mail thread which I
don't have, so I can't follow the details of your proposal.  But I would
appreciate you confirming that these proposals won't place any
constraints whatsoever on how users and major modes can use narrowing.

You can also understand me being a bit concerned at the reference to CC
Mode.  ;-)

> - Multi-mode packages have been using narrowing for this purpose for
> years, so they won't have to change much.

They have used narrowing because that is the only tool they have had.

> And, it's kind of funny, mhtml-indent-line which we've reviewed and
> added recently, uses narrow-to-region too. :-) While disregarding the
> second element of prog-indentation-context (which it set to the value
> "widen all you want"). Which works well enough because SMIE doesn't
> widen, and I've removed the `widen' call from js-mode indentation code
> 3.5 years ago.

> I'm adding Tom and Vitalie to Cc. Maybe they have something to add.

--
Alan Mackenzie (Nuremberg, Germany).

Reply | Threaded
Open this post in threaded view
|

[SUSPECTED SPAM] Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Stefan Monnier
> Using narrowing for marking the bounds of a sub-mode is a bad thing,
> since it is likely to cause contention with other uses of narrowing.

Dmitry's solution to this threat is to say that indent-line-function
should always be called fully-widened, so there can be no "other uses of
narrowing" that can get in the way.

> It's not clear what is meant here, but mandating maintainers of major
> modes to use narrowing in a particular way is at best controversial, and
> probably will render many major modes non-functional.

It doesn't mandate that major modes use narrowing.  Quite the opposite:
it says "don't touch narrowing inside your indentation function (unless
you know what you're doing), because it's already setup for you".


        Stefan

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Dmitry Gutov
In reply to this post by Alan Mackenzie
Hi Alan,

On 11/30/17 9:46 PM, Alan Mackenzie wrote:

> Using narrowing for marking the bounds of a sub-mode is a bad thing,
> since it is likely to cause contention with other uses of narrowing.

Please give an example.

> It's not clear what is meant here,

To rephrase, don't widen in indent-line-function or
beginning-of-defun-function.

> but mandating maintainers of major
> modes to use narrowing in a particular way is at best controversial, and
> probably will render many major modes non-functional.

I don't see a reason why. Even more, it should be compatible with all
uses of narrowing that I know about.

> Narrowing belongs to users and major modes.

It can serve all.

> I am lacking the context of
> your post, which appears to start deep down in some mail thread which I
> don't have, so I can't follow the details of your proposal.  But I would
> appreciate you confirming that these proposals won't place any
> constraints whatsoever on how users and major modes can use narrowing.

Please see
http://lists.gnu.org/archive/html/emacs-devel/2017-11/msg00691.html and
http://lists.gnu.org/archive/html/emacs-devel/2016-12/msg00796.html.

> You can also understand me being a bit concerned at the reference to CC
> Mode.  ;-)

You shouldn't be: CC Mode can just ignore this new rule, as long as it's
too hard to support embedding it in multi-mode buffers.

>> - Multi-mode packages have been using narrowing for this purpose for
>> years, so they won't have to change much.
>
> They have used narrowing because that is the only tool they have had.

The only other tool we have is also narrowing, semantically.

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Alan Mackenzie
Hello, Dmitry.

Just to be clear, in the following, by "narrowing" I mean all operations
of narrowing and widening regarded generically.

On Thu, Nov 30, 2017 at 23:09:00 +0000, Dmitry Gutov wrote:
> Hi Alan,

> On 11/30/17 9:46 PM, Alan Mackenzie wrote:

> > Using narrowing for marking the bounds of a sub-mode is a bad thing,
> > since it is likely to cause contention with other uses of narrowing.

> Please give an example.

A low level function which as an essential part of its functionality
(for example, to make sure point-min isn't within a comment or string)
widens.

> > It's not clear what is meant here,

> To rephrase, don't widen in indent-line-function or
> beginning-of-defun-function.

This is an intolerable restriction.  The low level function mentioned
above cannot, should not, must not know whether it's being called
(indirectly) from indent-line-function or b-o-d-function.  It will have
to widen in all cases or none.  Therefore there will be failures whilst
being called either from one of the two noted functions or from
elsewhere.

> > but mandating maintainers of major
> > modes to use narrowing in a particular way is at best controversial, and
> > probably will render many major modes non-functional.

> I don't see a reason why. Even more, it should be compatible with all
> uses of narrowing that I know about.

See above.

> > Narrowing belongs to users and major modes.

> It can serve all.

Indeed it can, and it must.  A super-mode thus may not "reserve"
narrowing for its own purposes to the exclusion of other uses.

[ .... ]

> > You can also understand me being a bit concerned at the reference to CC
> > Mode.  ;-)

> You shouldn't be: CC Mode can just ignore this new rule, as long as it's
> too hard to support embedding it in multi-mode buffers.

The multi-mode mechanism should be designed to be usable with any major
mode.  There's nothing particularly hard about supporting CC Mode in a
well designed multi-mode scheme.

> >> - Multi-mode packages have been using narrowing for this purpose for
> >> years, so they won't have to change much.

> > They have used narrowing because that is the only tool they have had.

> The only other tool we have is also narrowing, semantically.

We need better tools.  I have already proposed and offered to implement
such tools.

--
Alan Mackenzie (Nuremberg, Germany).

Reply | Threaded
Open this post in threaded view
|

Re: [SUSPECTED SPAM] Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Alan Mackenzie
In reply to this post by Stefan Monnier
Hello, Stefan.

On Thu, Nov 30, 2017 at 18:03:05 -0500, Stefan Monnier wrote:
> > Using narrowing for marking the bounds of a sub-mode is a bad thing,
> > since it is likely to cause contention with other uses of narrowing.

> Dmitry's solution to this threat is to say that indent-line-function
> should always be called fully-widened, .....

That is a horrible, confusing, restriction, and an incompatibility with
older and other Emacsen.

> .... so there can be no "other uses of narrowing" that can get in the
> way.

The indent-line-function will be calling low-level functions which
themselves widen, possibly to determine whether point-min is in a
literal, or more generally to determine context.  These low-level
functions will also be called from other contexts, hence the necessity
of widening.

> > It's not clear what is meant here, but mandating maintainers of major
> > modes to use narrowing in a particular way is at best controversial, and
> > probably will render many major modes non-functional.

> It doesn't mandate that major modes use narrowing.  Quite the opposite:
> it says "don't touch narrowing inside your indentation function (unless
> you know what you're doing), because it's already setup for you".

That is mandating the way major modes use (or don't use) narrowing.
It's entirely the wrong thing to do, and will lead to trouble.  Whatever
this abuse of narrowing is trying to achieve can be done in other less
damaging ways.

>         Stefan

--
Alan Mackenzie (Nuremberg, Germany).

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Stefan Monnier
In reply to this post by Alan Mackenzie
>> To rephrase, don't widen in indent-line-function or
>> beginning-of-defun-function.
> This is an intolerable restriction.  The low level function mentioned
> above cannot, should not, must not know whether it's being called
> (indirectly) from indent-line-function or b-o-d-function.

It's a change.  So, to make it work right, some code will need to adapt.
As mentioned by Dmitry, this shouldn't introduce breakage in general:
the code will only need to adapt *if* it wants to work with
multi-major-mode solutions.  For the normal use case, you can still
widen/narrow to your heart's content and nobody will be the wiser.

There's nothing special here: any multi-major-mode solution will need to
define some convention/protocol that major need to follow in order to
work well.

Now if you worry that (after the code is adapted) the result is code
that is too complex/brittle, time will tell, but I get the impression
that it won't be too bad, likely no worse than any other solution.

The way your above situation would be handled is to say "the low level
function mentioned above assumes that the buffer has already been
properly widened".  This way, it doesn't need to know whether it's being
called (indirectly) from indent-line-function or b-o-d-function.

But some of its callers may need to be changed to do the widening, and
indeed, in order to know how far to widen, they'll need the help from
the multi-major-mode package.

So maybe we need something like:

    (defvar prog-widen-function #'widen)
    (defun prog-widen () (funcall prog-widen-function))

[ And maybe for the same reason, we need a function (rather than
  a variable) that returns the `first-column`, so we can get the info
  anywhere/anytime rather than only during indentation.  ]


        Stefan

Reply | Threaded
Open this post in threaded view
|

RE: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Drew Adams
In reply to this post by Alan Mackenzie
> > > Narrowing belongs to users and major modes.
> > It can serve all.
>
> Indeed it can, and it must.  A super-mode thus may not "reserve"
> narrowing for its own purposes to the exclusion of other uses.

+1

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Stefan Monnier
In reply to this post by Alan Mackenzie
> Indeed it can, and it must.  A super-mode thus may not "reserve"
> narrowing for its own purposes to the exclusion of other uses.

99% of the multi-major-modes out there use narrowing for that purpose,
and in practice it doesn't restrict other uses of narrowing.

I've opposed the use of narrowing for that in the past, but the evidence
against that opposition is overwhelming.  Let's do that and move on.

It's not perfect, but we've already looked much too long for a less
imperfect solution.


        Stefan

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Alan Mackenzie
In reply to this post by Stefan Monnier
Hello, Stefan.

On Fri, Dec 01, 2017 at 11:26:08 -0500, Stefan Monnier wrote:
> >> To rephrase, don't widen in indent-line-function or
> >> beginning-of-defun-function.
> > This is an intolerable restriction.  The low level function mentioned
> > above cannot, should not, must not know whether it's being called
> > (indirectly) from indent-line-function or b-o-d-function.

> It's a change.  So, to make it work right, some code will need to adapt.
> As mentioned by Dmitry, this shouldn't introduce breakage in general:

That has yet to be shown.

> the code will only need to adapt *if* it wants to work with
> multi-major-mode solutions.  For the normal use case, you can still
> widen/narrow to your heart's content and nobody will be the wiser.

If we're going to have a multi-major-mode solution (and I think we
should), let's at least get it right.  I.e., it should work with _any_
major mode without grotesque restrictions on that mode.

I would very much like to have AWK Mode working inside a
shell-script-mode buffer.  Why not?  The MMM solution which appears to
be being proposed won't achieve this.

I would very much like the option of handling C buffers as MMM buffers,
where macros would be handled by a (slightly) different mode from normal
code.  I'm not sure whether this is a good or practicable idea, but it
would be nice to have the option.

> There's nothing special here: any multi-major-mode solution will need to
> define some convention/protocol that major need to follow in order to
> work well.

No.  Our MMM solution should work without imposing restrictions on the
major modes.  All the awkwardness should be dealt with by the MMM.  This
is possible.

> Now if you worry that (after the code is adapted) the result is code
> that is too complex/brittle, time will tell, but I get the impression
> that it won't be too bad, likely no worse than any other solution.

As I keep saying, we should adapt a solution that doesn't place manacles
on hackers.

> The way your above situation would be handled is to say "the low level
> function mentioned above assumes that the buffer has already been
> properly widened".  This way, it doesn't need to know whether it's being
> called (indirectly) from indent-line-function or b-o-d-function.

Then it ceases to be a general low level function and becomes one that
can only be called in certain circumstances.  This effectively transmits
high level state to the low levels, and isn't good software engineering.

> But some of its callers may need to be changed to do the widening, and
> indeed, in order to know how far to widen, they'll need the help from
> the multi-major-mode package.

This is intolerable.  It will lead to twisted code.

Why should a major mode need to know "how far to widen"?  This is only
due to the proposal to misuse narrowing to control regions with
different syntax-tables, I think.  If these regions are controlled
without using narrowing, this and many other problems cease to exist.

> So maybe we need something like:

>     (defvar prog-widen-function #'widen)
>     (defun prog-widen () (funcall prog-widen-function))

No, we need

    (widen)

like we've had for many decades.

> [ And maybe for the same reason, we need a function (rather than
>   a variable) that returns the `first-column`, so we can get the info
>   anywhere/anytime rather than only during indentation.  ]


>         Stefan

--
Alan Mackenzie (Nuremberg, Germany).

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Dmitry Gutov
In reply to this post by Stefan Monnier
On 12/1/17 4:26 PM, Stefan Monnier wrote:
> The way your above situation would be handled is to say "the low level
> function mentioned above assumes that the buffer has already been
> properly widened".  This way, it doesn't need to know whether it's being
> called (indirectly) from indent-line-function or b-o-d-function.

That one way, I'd call it "clean". But the change might take quite a bit
of effort, depending on the size of the code base.

> But some of its callers may need to be changed to do the widening, and
> indeed, in order to know how far to widen, they'll need the help from
> the multi-major-mode package.

That would be the "dirty" way.

> So maybe we need something like:
>
>      (defvar prog-widen-function #'widen)
>      (defun prog-widen () (funcall prog-widen-function))

I don't know if we need that in the common API, but individual packages
can define such shims if they find that convenient.

E.g.

(defvar cc-mode-inside-indent-line nil)

(defun cc-mode-maybe-widen ()
   ;; You can also throw in an Emacs version check here,
   ;; for good measure.
   (unless cc-mode-inside-indent-line (widen)))

And then the "low-level primitives" can call cc-mode-maybe-widen.

> [ And maybe for the same reason, we need a function (rather than
>    a variable) that returns the `first-column`, so we can get the info
>    anywhere/anytime rather than only during indentation.  ]

Please elaborate.

How does using a function instead of a variable change the duration when
prog-first-column is set?

And when else will we need this value?

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Dmitry Gutov
In reply to this post by Alan Mackenzie
On 12/1/17 3:49 PM, Alan Mackenzie wrote:

>> Please give an example.
>
> A low level function which as an essential part of its functionality
> (for example, to make sure point-min isn't within a comment or string)
> widens.

See Stefan and I's responses. Including an example of a shim that
wouldn't require you to change your code too much.

But first and foremost, CC Mode probably doesn't need to worry about
complying with this guideline (yet). In my experience, it doesn't work
well in multi-mode context for reasons other than it calling widen. So
worrying about this change in protocol might be premature.

>> To rephrase, don't widen in indent-line-function or
>> beginning-of-defun-function.
>
> This is an intolerable restriction.  The low level function mentioned
> above cannot, should not, must not know whether it's being called
> (indirectly) from indent-line-function or b-o-d-function.  It will have
> to widen in all cases or none.  Therefore there will be failures whilst
> being called either from one of the two noted functions or from
> elsewhere.

See that shim in the other email.

> Indeed it can, and it must.  A super-mode thus may not "reserve"
> narrowing for its own purposes to the exclusion of other uses.

Right.

> The multi-mode mechanism should be designed to be usable with any major
> mode.  There's nothing particularly hard about supporting CC Mode in a
> well designed multi-mode scheme.

It's harder than supporting Ruby, CSS and JS modes, that I can tell you
for sure. For one thing, the extra caches CC Mode uses have given me
multiple "argument out of bounds" errors when used with narrowing.

So these are my takeaways:

1) The "low-level primitives" in CC Mode do not widen consistently. Some
of them miss that responsibility (or else I wouldn't get those kind of
errors). That tells me that moving (widen) call to some higher level is
generally a good idea, but you can disagree.

2) The other errors should be solvable, but they require a separate
investigation, which nobody has found the time and energy to do so far,
over many years. So maybe it's not so necessary.

3) The solution to the other problems is most likely orthogonal to the
current discussion. Allowing multi-mode packages to use narrowing
certainly shouldn't hurt.

> We need better tools.  I have already proposed and offered to implement
> such tools.

It was much more complex, nowhere near even a proof of concept, and
doesn't seem to move anywhere, so far.

The change we're discussing is much smaller and pretty much implemented.

And again, there's no reason why they couldn't coexist.

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Stefan Monnier
In reply to this post by Dmitry Gutov
>> (defvar prog-widen-function #'widen)
>> (defun prog-widen () (funcall prog-widen-function))
> I don't know if we need that in the common API, but individual packages can
> define such shims if they find that convenient.

If we don't add such a thing in the common API, how could individual
packages know the boundary of the current chunk without adding
MMM-specific hacks?

> (defvar cc-mode-inside-indent-line nil)
> (defun cc-mode-maybe-widen ()
>   ;; You can also throw in an Emacs version check here,
>   ;; for good measure.
>   (unless cc-mode-inside-indent-line (widen)))
> And then the "low-level primitives" can call cc-mode-maybe-widen.

But just `widen` will do thew wrong thing (it'll widen too much) when
we're inside an MMM chunk.

>> [ And maybe for the same reason, we need a function (rather than
>> a variable) that returns the `first-column`, so we can get the info
>> anywhere/anytime rather than only during indentation.  ]
> Please elaborate.
> How does using a function instead of a variable change the duration when
> prog-first-column is set?

Having it as a function means you don't have to compute the value of
prog-first-column every time to "enter" a chunk, but instead we only
compute it if/when the submode decides it needs to know what is the
value of prog-first-column.

> And when else will we need this value?

Here are some examples I can think of:
- auto-fill-mode may like to compute the hypothetical indentation that
  would result from inserting a newline somewhere (and do that before
  touching the buffer).
- some package may like to highlight lines which aren't currently
  indented right (so, it won't call indent-according-to-mode, but
  it will need to compute the "desired" indentation).
I'm sure there can be other cases.


        Stefan

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Dmitry Gutov
On 12/1/17 7:51 PM, Stefan Monnier wrote:
>>> (defvar prog-widen-function #'widen)
>>> (defun prog-widen () (funcall prog-widen-function))
>> I don't know if we need that in the common API, but individual packages can
>> define such shims if they find that convenient.
>
> If we don't add such a thing in the common API, how could individual
> packages know the boundary of the current chunk without adding
> MMM-specific hacks?

The idea for multi-mode packages has always been to make do without
making the major modes actually aware of hunk boundaries.

And the snippet of code won't make them aware either, since the chunk
boundaries are conveyed via narrowing only in particular dynamic
contexts, such as when indent-line-functions is called.

Adding other means too might make sense, but that's expanding the scope
of the current proposal. I'd rather make it small and try to get it into
Emacs 26.

>> (defvar cc-mode-inside-indent-line nil)
>> (defun cc-mode-maybe-widen ()
>>    ;; You can also throw in an Emacs version check here,
>>    ;; for good measure.
>>    (unless cc-mode-inside-indent-line (widen)))
>> And then the "low-level primitives" can call cc-mode-maybe-widen.
>
> But just `widen` will do thew wrong thing (it'll widen too much) when
> we're inside an MMM chunk.

Yes, so CC Mode primitives all should call cc-mode-maybe-widen. Having
them call prog-widen vs widen is very much the same difference.

> Having it as a function means you don't have to compute the value of
> prog-first-column every time to "enter" a chunk, but instead we only
> compute it if/when the submode decides it needs to know what is the
> value of prog-first-column.

You probably mean adding a variable prog-first-column-function, then?

Or will we expect multi-mode packages to simply advise the
prog-first-column function? I'm fine with that.

>> And when else will we need this value?
>
> Here are some examples I can think of:
> - auto-fill-mode may like to compute the hypothetical indentation that
>    would result from inserting a newline somewhere (and do that before
>    touching the buffer).

Won't it need to call indent-line-function to find out how much the next
line should be indented?

> - some package may like to highlight lines which aren't currently
>    indented right (so, it won't call indent-according-to-mode, but
>    it will need to compute the "desired" indentation).

This example will *definitely* need to call indent-line-function.

And both of them should be solved by exchanging indent-line-function for
(non-mutating) line-intentation-function. But that's a change for
another day.

> I'm sure there can be other cases.

I'm sure there can be. A complete proposal to let the modes know chunk
boundaries, etc, has yet to be finalized, however. And just having
font-lock and indentation work reliably in multi-mode contexts will be a
significant win.

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Vitalie Spinu-2
In reply to this post by Stefan Monnier

Please correct me if I am wrong but the current patch requires all modes to use
`prog-widen` and reserve `widen` to the interactive use, right? If so, this is
the opposite of the older idea of adding `hard-limits` for widening which only
multi-modes should use.

I have proposed a patch for `set-widen-limits` 1.5 years ago which still lives
in scratch/hard-narrow branch. It turned to be a pretty straightforward feat and
worked without requiring any changes of the excising elisp code.

The lengthy discussion started here:

  https://lists.gnu.org/archive/html/emacs-devel/2016-04/msg00629.html

Somewhere along the way we decided in favor of adding an extra `hard` argument
to `narrow` instead of having a new `set-widen-limits` function. That turned to
be impossible as it required backward-incompatible changes to the byte-code. I
run out of steam by then and the whole idea died :(

If adding a new function is back on the table then I think `set-widen-limits` is
a better approach than `prog-widen`. It doesn't impose new restrictions on the
existing code and it avoids introducing new types of narrowing or widening.

Unless something changed in the meanwhile, the changes to the C code to make
that work were minimal. If there is interest I can get it another shot.

  Vitalie

Reply | Threaded
Open this post in threaded view
|

Re: [Emacs-diffs] scratch/widen-less a4ba846: Replace prog-widen with consolidating widen calls

Dmitry Gutov
On 12/1/17 9:27 PM, Vitalie Spinu wrote:
>
> Please correct me if I am wrong but the current patch requires all modes to use
> `prog-widen` and reserve `widen` to the interactive use, right? If so, this is
> the opposite of the older idea of adding `hard-limits` for widening which only
> multi-modes should use.

Quite the opposite: the patch under discussion, in scratch/widen-less,
removes prog-widen altogether.

> I have proposed a patch for `set-widen-limits` 1.5 years ago which still lives
> in scratch/hard-narrow branch. It turned to be a pretty straightforward feat and
> worked without requiring any changes of the excising elisp code.
>
> The lengthy discussion started here:
>
>    https://lists.gnu.org/archive/html/emacs-devel/2016-04/msg00629.html
>
> Somewhere along the way we decided in favor of adding an extra `hard` argument
> to `narrow` instead of having a new `set-widen-limits` function. That turned to
> be impossible as it required backward-incompatible changes to the byte-code. I
> run out of steam by then and the whole idea died :(

If that was the only difficulty, having set-widen-limits a separate
function would have been a better choice back then (I wasn't aware, FWIW).

The new proposal is a bit simpler, OTOH.

1234 ... 9