Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Dmitry Gutov]

A bit of a problem with this change is the syntax-ppss doscstring 
originally said (and still contains these words):

   except that values at positions 2 and 6
   in the returned list (counting from 0) cannot be relied upon.

So documenting them in this function is... not logical?

On 30.10.2019 14:16, Lars Ingebrigtsen wrote:
> branch: master
> commit 305dbc7e2be05748039aacb1a3d697f6f64bed4c
> Author: Lars Ingebrigtsen <larsi@gnus.org>
> Commit: Lars Ingebrigtsen <larsi@gnus.org>
> 
>      Move description of value to syntax-ppss function.
>      
>      * lisp/emacs-lisp/syntax.el (syntax-ppss): Move the description of
>      the return value from...
>      
>      * src/syntax.c (Fparse_partial_sexp): ... here because
>      `syntax-ppss' is what's called over the place, and jumping through
>      an indirection to get to the value description is inconvenient.
> ---
>   lisp/emacs-lisp/syntax.el | 20 ++++++++++++++++++++
>   src/syntax.c              | 20 +-------------------
>   2 files changed, 21 insertions(+), 19 deletions(-)
> 
> diff --git a/lisp/emacs-lisp/syntax.el b/lisp/emacs-lisp/syntax.el
> index 3861b16..913e34d 100644
> --- a/lisp/emacs-lisp/syntax.el
> +++ b/lisp/emacs-lisp/syntax.el
> @@ -476,6 +476,26 @@ run from `point-min' to POS except that values at positions 2 and 6
>   in the returned list (counting from 0) cannot be relied upon.
>   Point is at POS when this function returns.
>   
> +Value is a list of elements describing final state of parsing:
> + 0. depth in parens.
> + 1. character address of start of innermost containing list; nil if none.
> + 2. character address of start of last complete sexp terminated.
> + 3. non-nil if inside a string.
> +    (it is the character that will terminate the string,
> +     or t if the string should be terminated by a generic string delimiter.)
> + 4. nil if outside a comment, t if inside a non-nestable comment,
> +    else an integer (the current comment nesting).
> + 5. t if following a quote character.
> + 6. the minimum paren-depth encountered during this scan.
> + 7. style of comment, if any.
> + 8. character address of start of comment or string; nil if not in one.
> + 9. List of positions of currently open parens, outermost first.
> +10. When the last position scanned holds the first character of a
> +    (potential) two character construct, the syntax of that position,
> +    otherwise nil.  That construct can be a two character comment
> +    delimiter or an Escaped or Char-quoted character.
> +11..... Possible further internal information used by ‘parse-partial-sexp’.
> +
>   It is necessary to call `syntax-ppss-flush-cache' explicitly if
>   this function is called while `before-change-functions' is
>   temporarily let-bound, or if the buffer is modified without
> diff --git a/src/syntax.c b/src/syntax.c
> index edfdae2..8509213 100644
> --- a/src/syntax.c
> +++ b/src/syntax.c
> @@ -3530,25 +3530,7 @@ Parsing stops at TO or when certain criteria are met;
>   If fifth arg OLDSTATE is omitted or nil,
>    parsing assumes that FROM is the beginning of a function.
>   
> -Value is a list of elements describing final state of parsing:
> - 0. depth in parens.
> - 1. character address of start of innermost containing list; nil if none.
> - 2. character address of start of last complete sexp terminated.
> - 3. non-nil if inside a string.
> -    (it is the character that will terminate the string,
> -     or t if the string should be terminated by a generic string delimiter.)
> - 4. nil if outside a comment, t if inside a non-nestable comment,
> -    else an integer (the current comment nesting).
> - 5. t if following a quote character.
> - 6. the minimum paren-depth encountered during this scan.
> - 7. style of comment, if any.
> - 8. character address of start of comment or string; nil if not in one.
> - 9. List of positions of currently open parens, outermost first.
> -10. When the last position scanned holds the first character of a
> -    (potential) two character construct, the syntax of that position,
> -    otherwise nil.  That construct can be a two character comment
> -    delimiter or an Escaped or Char-quoted character.
> -11..... Possible further internal information used by `parse-partial-sexp'.
> +See `syntax-ppss' for a description of the return value.
>   
>   If third arg TARGETDEPTH is non-nil, parsing stops if the depth
>   in parentheses becomes equal to TARGETDEPTH.
> 

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Lars Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> A bit of a problem with this change is the syntax-ppss doscstring
> originally said (and still contains these words):
>
>   except that values at positions 2 and 6
>   in the returned list (counting from 0) cannot be relied upon.
>
> So documenting them in this function is... not logical?

It's slightly illogical, but it's documented that they "can't be relied
upon", whatever that means.

And splitting up the documentation over the two functions seemed even
worse.

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

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Dmitry Gutov]

On 30.10.2019 17:03, Lars Ingebrigtsen wrote:
> It's slightly illogical, but it's documented that they "can't be relied
> upon", whatever that means.

Point is, in parse-partial-sexp the *can* be relied upon. Though I'm not 
sure how often they are used.

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Lars Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Point is, in parse-partial-sexp the *can* be relied upon. Though I'm
> not sure how often they are used.

Yes, the doc string certainly implies that they can be relied upon if
you use that function instead.

On the other hand, perhaps I shouldn't have moved it --
parse-partial-sexp is used a lot, too.  I was only seeing syntax-ppss
everywhere, but I see that it's used less than 2x as much.

So perhaps that patch should be reverted anyway.  Or have the list in
both functions.  Or, even better, stop saying

(if (nth 5 state)
    (do-something-incomprehensible)
  (do-something-else-incomprehensible))

everywhere and just add accessor functions already, so that the code
becomes marginally more readable.  Because trying to make sense of it
now is just too hard, and it doesn't have to be.

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

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Dmitry Gutov]

On 30.10.2019 17:22, Lars Ingebrigtsen wrote:

> Yes, the doc string certainly implies that they can be relied upon if
> you use that function instead.
> 
> On the other hand, perhaps I shouldn't have moved it --
> parse-partial-sexp is used a lot, too.  I was only seeing syntax-ppss
> everywhere, but I see that it's used less than 2x as much.

syntax-ppss is indeed the function that most consumers should call, but 
it's low-level enough that I never considered that to be a problem.

> So perhaps that patch should be reverted anyway.

That's what I would do.

> Or have the list in
> both functions.  Or, even better, stop saying
> 
> (if (nth 5 state)
>      (do-something-incomprehensible)
>    (do-something-else-incomprehensible))
> 
> everywhere and just add accessor functions already, so that the code
> becomes marginally more readable.  Because trying to make sense of it
> now is just too hard, and it doesn't have to be.

Someone should probably try to add that accessor thing, but also 
perf-test the result.

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Lars Ingebrigtsen]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> So perhaps that patch should be reverted anyway.  Or have the list in
> both functions.  Or, even better, stop saying
>
> (if (nth 5 state)
>     (do-something-incomprehensible)
>   (do-something-else-incomprehensible))
>
> everywhere and just add accessor functions already, so that the code
> becomes marginally more readable.  Because trying to make sense of it
> now is just too hard, and it doesn't have to be.

If we want to do this, I volunteer to start doing the rewrite -- I have
some functions for this from the decoded time makeover...

So what would the accessor macros be?  Err...  looking at the doc
string, something like:

ppss-depth
ppss-start-innermost
ppss-start-last-complete-sexp
ppss-string-terminator
ppss-comment-nesting
ppss-after-quote-character
ppss-minimum-paren-depth
ppss-comment-style
ppss-start-comment-or-string
ppss-open-paren-positions
ppss-two-character-syntax
ppss-internal

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

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Lars Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Someone should probably try to add that accessor thing, but also
> perf-test the result.

The accessors would be macros, so the performance impact should be nil.

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

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Dmitry Gutov]

On 30.10.2019 17:29, Lars Ingebrigtsen wrote:
> If we want to do this, I volunteer to start doing the rewrite -- I have
> some functions for this from the decoded time makeover...
> 
> So what would the accessor macros be?  Err...  looking at the doc
> string, something like:
> 
> ppss-depth
> ppss-start-innermost
> ppss-start-last-complete-sexp
> ppss-string-terminator
> ppss-comment-nesting
> ppss-after-quote-character
> ppss-minimum-paren-depth
> ppss-comment-style
> ppss-start-comment-or-string
> ppss-open-paren-positions
> ppss-two-character-syntax
> ppss-internal

FWIW, we already have some accessors added, though they don't cover all 
of the fields (and none of these are macros):

syntax-ppss-depth
syntax-ppss-toplevel-pos
syntax-ppss-context

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Lars Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> FWIW, we already have some accessors added, though they don't cover
> all of the fields (and none of these are macros):
>
> syntax-ppss-depth
> syntax-ppss-toplevel-pos
> syntax-ppss-context

Only the first of these is an accessor -- the other two look like
utility functions.

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

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Dmitry Gutov]

On 30.10.2019 18:17, Lars Ingebrigtsen wrote:
> Only the first of these is an accessor -- the other two look like
> utility functions.

OK, you are right.

Regarding the names, some suggestions:

ppss-after-quote-character -> -after-quote-p
ppss-start-comment-or-string -> -comment-or-string-start
ppss-internal -> nix

ppss-start-last-complete-sexp and ppss-minimum-paren-depth can probably 
be nixed as well since "they can't be relied on".

And prepend the names with syntax-, I think.

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Lars Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Regarding the names, some suggestions:
>
> ppss-after-quote-character -> -after-quote-p
> ppss-start-comment-or-string -> -comment-or-string-start
> ppss-internal -> nix

Makes sense.

> ppss-start-last-complete-sexp and ppss-minimum-paren-depth can
> probably be nixed as well since "they can't be relied on".

But they can be relied on when returned from parse-partial-sexp.

> And prepend the names with syntax-, I think.

That makes them very long, which is awkward...  Perhaps just syntax-*
instead of syntax-ppss-* or ppss-*?

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

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Stefan Monnier]

> ppss-depth
> ppss-start-innermost
> ppss-start-last-complete-sexp
> ppss-string-terminator
> ppss-comment-nesting
> ppss-after-quote-character
> ppss-minimum-paren-depth
> ppss-comment-style
> ppss-start-comment-or-string
> ppss-open-paren-positions
> ppss-two-character-syntax
> ppss-internal

Yes, please start with (cl-defstruct (ppss (type list)) ...)


        Stefan

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Lars Ingebrigtsen]

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

>> ppss-depth
>> ppss-start-innermost
>> ppss-start-last-complete-sexp
>> ppss-string-terminator
>> ppss-comment-nesting
>> ppss-after-quote-character
>> ppss-minimum-paren-depth
>> ppss-comment-style
>> ppss-start-comment-or-string
>> ppss-open-paren-positions
>> ppss-two-character-syntax
>> ppss-internal
>
> Yes, please start with (cl-defstruct (ppss (type list)) ...)

Done (with Dmitry's suggestions).  Everybody -- feel free to bikeshed
(and change any of the accessor names) before we start using these.

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

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Alan Mackenzie]

Hello, Lars.

On Wed, Oct 30, 2019 at 16:29:44 +0100, Lars Ingebrigtsen wrote:
> Lars Ingebrigtsen <larsi@gnus.org> writes:

> > So perhaps that patch should be reverted anyway.  Or have the list in
> > both functions.  Or, even better, stop saying

> > (if (nth 5 state)
> >     (do-something-incomprehensible)
> >   (do-something-else-incomprehensible))

> > everywhere and just add accessor functions already, so that the code
> > becomes marginally more readable.  Because trying to make sense of it
> > now is just too hard, and it doesn't have to be.

> If we want to do this, I volunteer to start doing the rewrite -- I have
> some functions for this from the decoded time makeover...

> So what would the accessor macros be?  Err...  looking at the doc
> string, something like:

> ppss-depth
> ppss-start-innermost
> ppss-start-last-complete-sexp
> ppss-string-terminator
> ppss-comment-nesting
> ppss-after-quote-character
> ppss-minimum-paren-depth
> ppss-comment-style
> ppss-start-comment-or-string
> ppss-open-paren-positions
> ppss-two-character-syntax
> ppss-internal

For a start, why the prefix "ppss-"?  The values are the results of
calling parse-partial-sexp (however indirectly), so "pps-" would be more
accurate, as well as being a character shorter.

But I'm thoroughly against this change.  It's bloat.  On a good day,

            (or (nth 3 s) (nth 4 s))

will easily fit onto a single line of code, often with room for a
comment such as "; in a string or comment.".

            (or (pps-string-terminator s) (pps-comment-nesting s))

is much less likely to do so.  So we end up with an extra line, whether
a continuation line added by redisplay, or a real extra line added by
the hacker.  In either case this is undesirable.

I doubt these macros will be easier to read than the use of nth.  They
are too long to be instantly recognised - the eye and the brain must
scan them piece by piece.  (nth 3 s) can be a mental atom, requiring no
effort.

In practice, by far most of the accesses to the state returned by
parse-partial-sexp are elements 3, 4, and 8, so anybody using
parse-partial-sexp quickly learns what these mean.  Others can be
explained by comments, if needed.  Anybody who doesn't recognise elts 3,
4, and 8 is probably best advised to read the pertinent manual page
anyway.

These new macro names would be a burden to learn and use, and a burden
on the Emacs Lisp manual.

Please don't do this.

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

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Dmitry Gutov]

On 30.10.2019 22:34, Alan Mackenzie wrote:
> In practice, by far most of the accesses to the state returned by
> parse-partial-sexp are elements 3, 4, and 8, so anybody using
> parse-partial-sexp quickly learns what these mean.

Right.

TBH, I'm not in any hurry to start using them myself. But I've seen 
others ask for these, so maybe it's a good thing, at least to have them 
available.

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Alan Mackenzie]

Hello, Lars.

On Wed, Oct 30, 2019 at 16:22:50 +0100, Lars Ingebrigtsen wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:

> > Point is, in parse-partial-sexp the *can* be relied upon. Though I'm
> > not sure how often they are used.

> Yes, the doc string certainly implies that they can be relied upon if
> you use that function instead.

> On the other hand, perhaps I shouldn't have moved it --
> parse-partial-sexp is used a lot, too.  I was only seeing syntax-ppss
> everywhere, but I see that it's used less than 2x as much.

More to the point, parse-partial-sexp is a primitive, an essential
function upon which much is built.  It MUST be documented, including its
return value.  syntax-ppss is not an essential function, though it might
be necessary nowadays; it has had and still has its problems.

> So perhaps that patch should be reverted anyway.  Or have the list in
> both functions. ....

I think either alternative would be good.

[ .... ]

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

-- 
Alan Mackenzie (Nuremberg, Germany).

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Stefan Monnier]

> For a start, why the prefix "ppss-"?

Not sure why he chose it, but I chose "ppss" in "syntax-ppss" because it
stores a "parse-parial-sexp state".


        Stefan

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Lars Ingebrigtsen]

Alan Mackenzie <acm@muc.de> writes:

> For a start, why the prefix "ppss-"?  The values are the results of
> calling parse-partial-sexp (however indirectly), so "pps-" would be more
> accurate, as well as being a character shorter.

As syntax.el says:

;; Note: PPSS stands for `parse-partial-sexp state'

> I doubt these macros will be easier to read than the use of nth.  They
> are too long to be instantly recognised - the eye and the brain must
> scan them piece by piece.  (nth 3 s) can be a mental atom, requiring no
> effort.

No, (nth 3 s) can mean anything.  You first have to backtrack up the
code to see what s is, then go to the doc string for syntax-ppss, and
then go to the doc string for parse-partial-sexp to see what it means.

> In practice, by far most of the accesses to the state returned by
> parse-partial-sexp are elements 3, 4, and 8, so anybody using
> parse-partial-sexp quickly learns what these mean.  Others can be
> explained by comments, if needed.  Anybody who doesn't recognise elts 3,
> 4, and 8 is probably best advised to read the pertinent manual page
> anyway.

I think that's a kinda elitist way of looking at it.

If you don't understand obscure code, there's nothing wrong with you --
there's something wrong with the code.

Font locking is something normal users should be able to understand,
because it's something that's very visual, has an immediate impact, and
can be tweaked endlessly.  But reading the code today is much harder
than it has to be, and my guess is that many people will just bail when
presented with code like

      ;; Find each interesting place between here and `end'.
      (while
	  (progn
	    (when (or (nth 3 state) (nth 4 state))
	      (setq face (funcall font-lock-syntactic-face-function state))
              ...

I think we had the same discussion with decoded time accessors?  That
was a minor understandability hurdle compared to ppss -- at least there
you could sort of guess what they were because they were increasing
(minutes higher than seconds).  The ppss nths are completely inscrutable.

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

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Noam Postavsky]

On Wed, 30 Oct 2019 at 18:22, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Done (with Dmitry's suggestions).  Everybody -- feel free to bikeshed
> (and change any of the accessor names) before we start using these.

Is it too late to bikeshed the names? ;)
I had earlier proposed some slightly shorter ones in
https://debbugs.gnu.org/32504#51, posting them here as a diff.

@@ -90,30 +90,30 @@ syntax-propertize-extend-region-functions
                (:copier nil)
                (:type list))
   (depth nil :documentation "depth in parens")
-  (innermost-start
+  (list-start
    nil :documentation
    "character address of start of innermost containing list; nil if none.")
-  (last-complete-sexp-start
+  (last-sexp-start
    nil :documentation
    "character address of start of last complete sexp terminated.")
   (string-terminator nil :documentation "\
 non-nil if inside a string.
 (it is the character that will terminate the string, or t if the
 string should be terminated by a generic string delimiter.)")
-  (comment-nesting nil :documentation "\
+  (comment nil :documentation "\
 nil if outside a comment, t if inside a non-nestable comment,
 else an integer (the current comment nesting).")
-  (after-quote-p nil :documentation "t if following a quote character.")
-  (minimum-paren-depth
+  (quoted-p nil :documentation "t if following a quote character.")
+  (min-depth
    nil :documentation "the minimum paren-depth encountered during this scan.")
   (comment-style nil :documentation "style of comment, if any.")
-  (comment-or-string-start
+  (context-start
    nil :documentation
    "character address of start of comment or string; nil if not in one.")
-  (open-paren-positions
+  (open-parens
    nil :documentation
    "List of positions of currently open parens, outermost first.")
-  (two-character-syntax nil :documentation "\
+  (syntax-sequence nil :documentation "\
 When the last position scanned holds the first character of a
 (potential) two character construct, the syntax of that position,
 otherwise nil.  That construct can be a two character comment

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Dmitry Gutov]

On 13.12.2019 14:34, Noam Postavsky wrote:
> On Wed, 30 Oct 2019 at 18:22, Lars Ingebrigtsen <larsi@gnus.org> wrote:
> 
>> Done (with Dmitry's suggestions).  Everybody -- feel free to bikeshed
>> (and change any of the accessor names) before we start using these.
> 
> Is it too late to bikeshed the names? ;)
> I had earlier proposed some slightly shorter ones in
> https://debbugs.gnu.org/32504#51, posting them here as a diff.
> 
> @@ -90,30 +90,30 @@ syntax-propertize-extend-region-functions
>                  (:copier nil)
>                  (:type list))
>     (depth nil :documentation "depth in parens")
> -  (innermost-start
> +  (list-start

Sounds ambiguous: the point is that it's innermost, among possible other 
list starts.

>      nil :documentation
>      "character address of start of innermost containing list; nil if none.")
> -  (last-complete-sexp-start
> +  (last-sexp-start

Same (but, like, in reverse): "complete" is important.

>      nil :documentation
>      "character address of start of last complete sexp terminated.")
>     (string-terminator nil :documentation "\
>   non-nil if inside a string.
>   (it is the character that will terminate the string, or t if the
>   string should be terminated by a generic string delimiter.)")
> -  (comment-nesting nil :documentation "\
> +  (comment nil :documentation "\

Doesn't this name imply some other value? Like a string (comment opener 
or its contents)?

> -  (comment-or-string-start
> +  (context-start
>      nil :documentation
>      "character address of start of comment or string; nil if not in one.")

That kind of implies that strings and comments are the most important 
contexts when parsing a file.

> -  (open-paren-positions
> +  (open-parens
>      nil :documentation
>      "List of positions of currently open parens, outermost first.")
> -  (two-character-syntax nil :documentation "\
> +  (syntax-sequence nil :documentation "\
>   When the last position scanned holds the first character of a
>   (potential) two character construct, the syntax of that position,
>   otherwise nil.  That construct can be a two character comment

These look okay to me.

min-depth too, but, like last-complete-sexp-start, these fields in 
values returned by syntax-ppss are unreliable/undefined, so they won't 
be used in most Lisp programs anyway.

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Lars Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

>> -  (open-paren-positions
>> +  (open-parens
>>      nil :documentation
>>      "List of positions of currently open parens, outermost first.")
>> -  (two-character-syntax nil :documentation "\
>> +  (syntax-sequence nil :documentation "\
>>   When the last position scanned holds the first character of a
>>   (potential) two character construct, the syntax of that position,
>>   otherwise nil.  That construct can be a two character comment
>
> These look okay to me.

Yes, I think so, too.

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

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Noam Postavsky]

On Fri, 13 Dec 2019 at 08:03, Dmitry Gutov <dgutov@yandex.ru> wrote:

>> -  (innermost-start
>> +  (list-start
>
> Sounds ambiguous: the point is that it's innermost, among possible other
> list starts.

>> -  (last-complete-sexp-start
>> +  (last-sexp-start
>
> Same (but, like, in reverse): "complete" is important.

Not sure I agree, but I can live with the current names.

> > -  (comment-nesting nil :documentation "\
> > +  (comment nil :documentation "\
>
> Doesn't this name imply some other value? Like a string (comment opener
> or its contents)?

Hmm, you might be right about that. How about 'comment-depth': two
characters shorter, and the other names also use "depth" rather than
"nesting".

> > -  (comment-or-string-start
> > +  (context-start
> >      nil :documentation
> >      "character address of start of comment or string; nil if not in one.")
>
> That kind of implies that strings and comments are the most important
> contexts when parsing a file.

Yeah, I think was I looking at syntax-ppss-context when I originally
wrote this, but in that case there is an argument to tell what
"context" refers to, so I agree it doesn't really make sense here.

> > -  (open-paren-positions
> > +  (open-parens
> >      nil :documentation
> >      "List of positions of currently open parens, outermost first.")
> > -  (two-character-syntax nil :documentation "\
> > +  (syntax-sequence nil :documentation "\
> >   When the last position scanned holds the first character of a
> >   (potential) two character construct, the syntax of that position,
> >   otherwise nil.  That construct can be a two character comment
>
> These look okay to me.

I'm actually feeling that the two-character-syntax one should be left
as is, it's kind of obscure so having a longer and more explicit name
seems better.

> min-depth too, but, like last-complete-sexp-start, these fields in
> values returned by syntax-ppss are unreliable/undefined, so they won't
> be used in most Lisp programs anyway.

I might be biased by having worked on the lisp indentation code which
uses those fields quite a bit.

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Dmitry Gutov]

On 15.02.2020 16:55, Noam Postavsky wrote:

>>> -  (comment-nesting nil :documentation "\
>>> +  (comment nil :documentation "\
>>
>> Doesn't this name imply some other value? Like a string (comment opener
>> or its contents)?
> 
> Hmm, you might be right about that. How about 'comment-depth': two
> characters shorter, and the other names also use "depth" rather than
> "nesting".

Sounds good.

>>> -  (open-paren-positions
>>> +  (open-parens
>>>       nil :documentation
>>>       "List of positions of currently open parens, outermost first.")
>>> -  (two-character-syntax nil :documentation "\
>>> +  (syntax-sequence nil :documentation "\
>>>    When the last position scanned holds the first character of a
>>>    (potential) two character construct, the syntax of that position,
>>>    otherwise nil.  That construct can be a two character comment
>>
>> These look okay to me.
> 
> I'm actually feeling that the two-character-syntax one should be left
> as is, it's kind of obscure so having a longer and more explicit name
> seems better.

Agree (I also have never used it).

>> min-depth too, but, like last-complete-sexp-start, these fields in
>> values returned by syntax-ppss are unreliable/undefined, so they won't
>> be used in most Lisp programs anyway.
> 
> I might be biased by having worked on the lisp indentation code which
> uses those fields quite a bit.

So... Lisp indentation code calls parse-partial-sexp directly?

Anyway, I said min-depth is okay, but if we're going to have 
comment-depth, maybe min-depth starts to sound more ambiguous.

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Noam Postavsky]

On Sat, 15 Feb 2020 at 10:14, Dmitry Gutov <dgutov@yandex.ru> wrote:

> >> min-depth too, but, like last-complete-sexp-start, these fields in
> >> values returned by syntax-ppss are unreliable/undefined, so they won't
> >> be used in most Lisp programs anyway.
> >
> > I might be biased by having worked on the lisp indentation code which
> > uses those fields quite a bit.
>
> So... Lisp indentation code calls parse-partial-sexp directly?

Yes, syntax-ppss isn't useful there because changing the indentation
invalidates the cache.

> Anyway, I said min-depth is okay, but if we're going to have
> comment-depth, maybe min-depth starts to sound more ambiguous.

I don't think so, unqualified "depth" refers to parens, as in the 0th field.

Re: master 305dbc7 2/4: Move description of value to syntax-ppss function.

[Noam Postavsky]

I've pushed my changes to emacs-27.

[1: ba7004b2a7]: 2020-02-23 09:03:18 -0500
  Shorten some ppss struct field names
  https://git.savannah.gnu.org/cgit/emacs.git/commit/?id=ba7004b2a74c69450114c12ef4521768fc165e8e