bug#59602: 29.0.50; csharp-mode should either be a package, or compat code could be removed

bug#59602: 29.0.50; csharp-mode should either be a package, or compat code could be removed

[Stefan Kangas]

Severity: wishlist

Is csharp-mode.el intended to be a :core package or should people just
use the version that comes with Emacs?

If the former, I guess some Version and Package-Requires headers
are needed.

If the latter, the compat code starting on line 512 could probably be
removed.

bug#59602: 29.0.50; csharp-mode should either be a package, or compat code could be removed

[Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Stefan Kangas <stefankangas@gmail.com> writes:

> Severity: wishlist
>
> Is csharp-mode.el intended to be a :core package or should people just
> use the version that comes with Emacs?
>
> If the former, I guess some Version and Package-Requires headers
> are needed.
>
> If the latter, the compat code starting on line 512 could probably be
> removed.

I agree.  The compat code should be removed.  I don't thing we have much
to gain by maintaining this as a :core package, so here's a patch doing
what you suggest.  I don't think that code should be part of Emacs at
all, if not absolutely necessary :-)

Theo


[-- Attachment #2: 0001-Remove-compatibility-code-in-csharp-mode.patch --]
[-- Type: text/x-diff, Size: 4466 bytes --]

From 44189f78889e22f3e9321d8b089ed452bfa3c87a Mon Sep 17 00:00:00 2001
From: Theodor Thornhill <theo@thornhill.no>
Date: Sat, 26 Nov 2022 20:10:58 +0100
Subject: [PATCH] Remove compatibility code in csharp-mode

* lisp/progmodes/csharp-mode.el (c-basic-matchers-before): Remove
invalid string check for Emacs 27.
(version=): Remove compatibility hack for string handling in CC Mode
in Emacs 27.1.
---
 lisp/progmodes/csharp-mode.el | 75 +----------------------------------
 1 file changed, 2 insertions(+), 73 deletions(-)

diff --git a/lisp/progmodes/csharp-mode.el b/lisp/progmodes/csharp-mode.el
index a544a4b5cb..354aacf9bc 100644
--- a/lisp/progmodes/csharp-mode.el
+++ b/lisp/progmodes/csharp-mode.el
@@ -319,12 +319,7 @@ csharp--color-forwards
 (c-lang-defconst c-basic-matchers-before
   csharp `(
            ;; Warning face on unclosed strings
-           ,@(if (version< emacs-version "27.0")
-                 ;; Taken from 26.1 branch
-                 `(,(c-make-font-lock-search-function
-                     (concat ".\\(" c-string-limit-regexp "\\)")
-                     '((c-font-lock-invalid-string))))
-               `(("\\s|" 0 font-lock-warning-face t nil)))
+           ("\\s|" 0 font-lock-warning-face t nil)
 
            ;; Invalid single quotes
            c-font-lock-invalid-single-quotes
@@ -346,7 +341,6 @@ c-basic-matchers-before
                   nil
                   (goto-char (match-end 0)))))
 
-
            ;; Negation character
            (eval . (list "\\(!\\)[^=]" 1 c-negation-char-face-name))
 
@@ -375,8 +369,7 @@ c-basic-matchers-before
            (eval . (list (concat "\\<catch\\> *( *"
                                  csharp--regex-type-name-matcher
                                  " *) *")
-                         1 font-lock-type-face))
-           ))
+                         1 font-lock-type-face))))
 
 (c-lang-defconst c-basic-matchers-after
   csharp (append
@@ -505,70 +498,6 @@ csharp-guess-basic-syntax
 
 ;;; End of new syntax constructs
 
-
-
-;;; Fix for strings on version 27.1
-
-(when (version= emacs-version "27.1")
-  ;; See:
-  ;; https://github.com/emacs-csharp/csharp-mode/issues/175
-  ;; https://github.com/emacs-csharp/csharp-mode/issues/151
-  ;; for the full story.
-  (defun c-pps-to-string-delim (end)
-    (let* ((start (point))
-           (no-st-s `(0 nil nil ?\" nil nil 0 nil ,start nil nil))
-           (st-s `(0 nil nil t nil nil 0 nil ,start nil nil))
-           no-st-pos st-pos
-           )
-      (parse-partial-sexp start end nil nil no-st-s 'syntax-table)
-      (setq no-st-pos (point))
-      (goto-char start)
-      (while (progn
-               (parse-partial-sexp (point) end nil nil st-s 'syntax-table)
-               (unless (bobp)
-                 (c-clear-syn-tab (1- (point))))
-               (setq st-pos (point))
-               (and (< (point) end)
-                    (not (eq (char-before) ?\")))))
-      (goto-char (min no-st-pos st-pos))
-      nil))
-
-  (defun c-multiline-string-check-final-quote ()
-    (let (pos-ll pos-lt)
-      (save-excursion
-        (goto-char (point-max))
-        (skip-chars-backward "^\"")
-        (while
-            (and
-             (not (bobp))
-             (cond
-              ((progn
-                 (setq pos-ll (c-literal-limits)
-                       pos-lt (c-literal-type pos-ll))
-                 (memq pos-lt '(c c++)))
-               ;; In a comment.
-               (goto-char (car pos-ll)))
-              ((save-excursion
-                 (backward-char)        ; over "
-                 (c-is-escaped (point)))
-               ;; At an escaped string.
-               (backward-char)
-               t)
-              (t
-               ;; At a significant "
-               (c-clear-syn-tab (1- (point)))
-               (setq pos-ll (c-literal-limits)
-                     pos-lt (c-literal-type pos-ll))
-               nil)))
-          (skip-chars-backward "^\""))
-        (cond
-         ((bobp))
-         ((eq pos-lt 'string)
-          (c-put-syn-tab (1- (point)) '(15)))
-         (t nil))))))
-
-;;; End of fix for strings on version 27.1
-
 ;; When invoked by MSBuild, csc’s errors look like this:
 ;; subfolder\file.cs(6,18): error CS1006: Name of constructor must
 ;; match name of class [c:\Users\user\project.csproj]
-- 
2.34.1

bug#59602: 29.0.50; csharp-mode should either be a package, or compat code could be removed

[Jostein Kjønigsen]

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

This may be a bit "on the side", but if me and Theodor are going to be 
maintaining csharp-mode in Emacs core, we currently face the slight 
"challenge" of not actually having write/push-access to the Emacs 
git-repo at savannah.

That means that at top of our ability, we will be able to email in 
patches (like Theodor did now!) and hope for someone to review and/or 
merge them.

While don't expect that providing for unlimited git-access out of the 
blue probably is how new major-modes are taken in, I'm also not sure 
what the "normal" procedure is in cases like this.

Are there someone particular we are recommended to CC in our emails to 
make sure the patches get picked up efficiently? And what if it's not a 
registered "bug" yet, but just an improvement? Should we send it to 
emacs-devel instead?

In short: Do you guys have any "onboarding guide" we can follow to make 
sure everyone is happy, and our patches get reviewed and applied 
efficiently? :)

Any guidance appreciated!

--
Kind regards
*Jostein Kjønigsen*

jostein@kjonigsen.net 🍵 jostein@gmail.com
https://jostein.kjønigsen.no <https://jostein.kjønigsen.no>
On 26.11.2022 20:12, Theodor Thornhill wrote:
> Stefan Kangas<stefankangas@gmail.com>  writes:
>
>> Severity: wishlist
>>
>> Is csharp-mode.el intended to be a :core package or should people just
>> use the version that comes with Emacs?
>>
>> If the former, I guess some Version and Package-Requires headers
>> are needed.
>>
>> If the latter, the compat code starting on line 512 could probably be
>> removed.
> I agree.  The compat code should be removed.  I don't thing we have much
> to gain by maintaining this as a :core package, so here's a patch doing
> what you suggest.  I don't think that code should be part of Emacs at
> all, if not absolutely necessary :-)
>
> Theo
>

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

bug#59602: 29.0.50; csharp-mode should either be a package, or compat code could be removed

[Eli Zaretskii]

> Date: Sat, 26 Nov 2022 21:58:35 +0100
> From: Jostein Kjønigsen <jostein@secure.kjonigsen.net>
> 
> While don't expect that providing for unlimited git-access out of the blue probably is how new major-modes
> are taken in, I'm also not sure what the "normal" procedure is in cases like this.

The "normal" procedure is to keep posting patches for review until you feel
you've been doing that long enough, and managed to get to the state where
your patches get only minor review comments, especially about coding and
documentation conventions.  Then request write access.

> Are there someone particular we are recommended to CC in our emails to make sure the patches get
> picked up efficiently? And what if it's not a registered "bug" yet, but just an improvement? Should we send it
> to emacs-devel instead?

It is always best to use report-emacs-bug for sending patches, even if it
isn't a bug.  debbugs is our issue tracker, not just bug tracker.
emacs-devel is for discussing ideas and asking questions (you can also post
diffs to better explain what you mean or if the question is about a
particular code).

TIA

bug#59602: 29.0.50; csharp-mode should either be a package, or compat code could be removed

[Stefan Kangas]

Theodor Thornhill <theo@thornhill.no> writes:

> I agree.  The compat code should be removed.  I don't thing we have much
> to gain by maintaining this as a :core package, so here's a patch doing
> what you suggest.  I don't think that code should be part of Emacs at
> all, if not absolutely necessary :-)

Thanks, since both you and Jostein seems to agree, I've pushed this
change (commit ea2f2f1e71).  I'm therefore closing this bug report.

bug#59602: 29.0.50; csharp-mode should either be a package, or compat code could be removed

[Theodor Thornhill via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On 27 November 2022 10:38:44 CET, Stefan Kangas <stefankangas@gmail.com> wrote:
>Theodor Thornhill <theo@thornhill.no> writes:
>
>> I agree.  The compat code should be removed.  I don't thing we have much
>> to gain by maintaining this as a :core package, so here's a patch doing
>> what you suggest.  I don't think that code should be part of Emacs at
>> all, if not absolutely necessary :-)
>
>Thanks, since both you and Jostein seems to agree, I've pushed this
>change (commit ea2f2f1e71).  I'm therefore closing this bug report.

Thanks, appreciate it :)

Theo