Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

elisp save-excursion #2513

Closed
spaette opened this issue Oct 9, 2024 · 14 comments
Closed

elisp save-excursion #2513

spaette opened this issue Oct 9, 2024 · 14 comments

Comments

@spaette
Copy link
Contributor

spaette commented Oct 9, 2024

@mavit

Is the sed output is referring to Emacs running in text terminals with truncation turned on?

$ sed -n '70,71p' ninja/misc/ninja-mode.el
                  ;; Except if the previous line is a comment as well, as the
                  ;; continuation dollar is ignored then.
$ 
@mavit
Copy link
Contributor

mavit commented Oct 9, 2024

I'm not sure why you're asking me in particular, but no, this is nothing to do with Emacs' display.

It's discussing the fact that, in a .ninja file, a $ at the end of the line is used to indicate that the next line is a continuation of the current line, except if the $ is part of a comment. https://ninja-build.org/manual.html#ref_lexer

Hope this helps.

@spaette spaette closed this as completed Oct 9, 2024
@spaette
Copy link
Contributor Author

spaette commented Oct 10, 2024

@jhasse

I've had the time to think about the previous comment and the manual's linked to section.

misc/ninja-mode.el

If the lisp function is treating code lines and comment lines differently this makes more sense to me.

                   (or
-                   ;; If we're continuing the previous line, it's not a
-                   ;; comment.
+                  ;; This is a code line.
                    (not (eq ?$ (char-before)))
-                   ;; Except if the previous line is a comment as well, as the
-                   ;; continuation dollar is ignored then.
+                   ;; This is a comment line, the escape character $ is ignored.
                  (nth 4 (syntax-ppss)))))

https://ninja-build.org/manual.html#ref_lexer

doc/manual.asciidoc

I think this might be an improvement in the manual.

  There is only one escape character, `$`, and it has the following
  behaviors:

  `$` followed by a newline:: escape the newline (continue the current line
  across a line break).
- 
- `$` followed by text:: a variable reference.
- 
- `${varname}`:: alternate syntax for `$varname`.
  
  `$` followed by space:: a space.  (This is only necessary in lists of
  paths, where a space would otherwise separate filenames.  See below.)
  
  `$:` :: a colon.  (This is only necessary in `build` lines, where a colon
  would otherwise terminate the list of outputs.)
  
  `$$`:: a literal `$`.
+ 
+ An unescaped `$` retains it's ordinary use in shell parameter
+ expansion.
  
  A `build` or `default` statement is first parsed as a space-separated
  list of filenames and then each name is expanded.  This means that
  spaces within a variable will result in spaces in the expanded
  filename.

@jhasse
Copy link
Collaborator

jhasse commented Oct 10, 2024

Regarding ninja/misc/ninja-mode.el or anythimg Emacs: Couldn't care less.

An unescaped $ retains it's ordinary use in shell parameter expansion.

I don't understand. This section is about Ninja's lexing, why does the shell have anything to do with it?

@spaette
Copy link
Contributor Author

spaette commented Oct 10, 2024

why does the shell have anything to do with it

...it seems I'm fatigued

The use of the escape character $ before the newline (as per the docs) in this .ninja file excerpt is obvious. What I was attempting to address is my reading of $builddir involves what for example the bash manual refers to as parameter expansion.

https://ninja-build.org/build.ninja.html

root = .
builddir = build
cxx = g++
ar = ar
cflags = -g -Wall -Wextra -Wno-deprecated -Wno-missing-field-initializers $
    -Wno-unused-parameter -fno-rtti -fno-exceptions -fvisibility=hidden $
    -pipe '-DNINJA_PYTHON="python"' -O2 -DNDEBUG -DUSE_PPOLL $
    -DNINJA_HAVE_BROWSE -I.
ldflags = -L$builddir

@musvaage
Copy link
Contributor

musvaage commented Oct 24, 2024

misc/ninja-mode.el

shorten to (end-of-line 0)

fixed as per @monnier comment which follows

                 (save-excursion
-                  (goto-char (line-end-position 0))
+                  (end-of-line 0) ; Go to the end of the previous line.
                   (or

(1+ (buffer-size)) should be replaced with (point-max)

              ;; Otherwise we get an `args-out-of-range' error.
-             (unless (= line-end (1+ (buffer-size)))
+             (unless (= line-end (point-max))
                (put-text-property line-end (1+ line-end) 'syntax-table '(12)))))))))

an author/maintainer might clarify why they don't change the syntax of $ to "escape" and of \n to "end of comment"

tho (and presumably set comment-end-can-be-escaped)

Emacs Lisp Reference Manual

Function: point-max
This function returns the maximum accessible value of point in the current buffer. This is (1+ (buffer-size)), unless narrowing is in effect, in which case it is the position of the end of the region that you narrowed to. (See Narrowing).

Command: end-of-line &optional count
This function moves point to the end of the current line. With an argument count not nil or 1, it moves forward count-1 lines and then to the end of the line.

This function does not move point across a field boundary (see Defining and Using Fields) unless doing so would move beyond there to a different line; therefore, if count is nil or 1, and point starts at a field boundary, point does not move. To ignore field boundaries, bind inhibit-field-text-motion to t.

If this function reaches the end of the buffer (or of the accessible portion, if narrowing is in effect), it positions point there. No error is signaled.

Function: line-end-position &optional count
Return the position that (end-of-line count) would move to.

Variable: comment-end-can-be-escaped
If this buffer local variable is non-nil, a single character which usually terminates a comment doesn’t do so when that character is escaped. This is used in C and C++ Modes, where line comments starting with ‘//’ can be continued onto the next line by escaping the newline with ‘\’.

@monnier
Copy link

monnier commented Oct 24, 2024

It shouldn't be (goto-char (end-of-line 0)) but (end-of-line 0). [ Also, I'd move the comment from "just before" (where it's not 100% clear whether it refers to just the next line or to the rest of the save-excursion), to right after (on the same line) with a shorter ; Go to the end of the previous line. ]

@mavit: Why did you say "except if the $ is part of a comment"? I don't see such an exception in the spec you pointed to. IOW, AFAICT, if a comment line ends in $, then the next line is still part of the comment.

@musvaage
Copy link
Contributor

@evmar

Are you OK with updating the ninja mode file?

@mavit
Copy link
Contributor

mavit commented Oct 25, 2024

@monnier asked:

@mavit: Why did you say "except if the $ is part of a comment"? I don't see such an exception in the spec you pointed to. IOW, AFAICT, if a comment line ends in $, then the next line is still part of the comment.

My understanding was that this takes priority:

Comments begin with # and extend to the end of the line.

@monnier
Copy link

monnier commented Oct 25, 2024 via email

@spaette
Copy link
Contributor Author

spaette commented Oct 25, 2024

cf: #2513 (comment)

doc/manual.asciidoc

Presenting "escape character, $," as exhibiting a behavior in regards to parameter expansion still seems off.

As it stands there seems now to have been added to this ticket clarification and references on how to update the file.

@evmar
Copy link
Collaborator

evmar commented Oct 26, 2024

@evmar

Are you OK with updating the ninja mode file?

I last touched this twelve years ago, I remember nothing!

Looks like @Hi-Angel is the most recent person to touch this file, so maybe they could provide a useful comment.

@Hi-Angel
Copy link
Contributor

@evmar
Are you OK with updating the ninja mode file?

I last touched this twelve years ago, I remember nothing!

Looks like @Hi-Angel is the most recent person to touch this file, so maybe they could provide a useful comment.

Please open a PR and link to it from here for review. I see multiple things being discussed in the thread, it'd be easier to review the actual code 😊 But I'm in favor of "end-of-line" and "point-max" changes if that's what the question is about.

FWIW, I'm pretty sure there's more to refactor, it's a pretty old code that's been touched by many different people. I personally only added indentation support to the mode, and fixed a minor highlighting bug. But I'd be happy to provide review, no problem.

@musvaage
Copy link
Contributor

musvaage commented Oct 26, 2024

consensus seems to be that "end-of-line" and "point-max" changes (#2513 (comment)) are to be implemented

doesn't matter who opens a pull on that

In any case if $ is given "escape" syntax, then this is simply
controlled by comment-end-can-be-escaped.

what remains is whether this quote is actionable, and if so how the elisp file would correspondingly be modified

@musvaage
Copy link
Contributor

more to refactor

@girzel

I'd be curious of your comments if you have the time to look over the code.

@musvaage musvaage mentioned this issue Oct 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants