NonGNU ELPA: Conditions for accepting a potential new package 'rmsbolt' ?

NonGNU ELPA: Conditions for accepting a potential new package 'rmsbolt' ?

[Jeremy Bryant]

I would like to recommend the package 'rmsbolt' for NonGNU ELPA and
volunteer for any required changes.

The author is Jay Kamat, who as I understand is potentially supportive
if the changes are minimal (and I am volunteering for these.)



1. What is it?
"A supercharged implementation of the [[https://github.com/mattgodbolt/compiler-explorer][godbolt compiler-explorer]] for Emacs.

RMSbolt tries to make it easy to see what your compiler is doing. It does this
by showing you the assembly output of a given source code file. It also
highlights which source code a given assembly block corresponds to, and vice
versa. It supports more types of languages than any previous tool of its kind.
"

It is currently hosted on gitlab and distributed on MELPA.



2. License
From reading nongnu.elpa.git's README.org

The package displays its License, which is Affero.
Is the GNU Affero license considered suitable for NonGNU ELPA?


3. Other
I believe it follows the other requirements.


WDYT?

Re: NonGNU ELPA: Conditions for accepting a potential new package 'rmsbolt' ?

[Philip Kaludercic]

Jeremy Bryant <jb@jeremybryant.net> writes:

> I would like to recommend the package 'rmsbolt' for NonGNU ELPA and
> volunteer for any required changes.
>
> The author is Jay Kamat, who as I understand is potentially supportive
> if the changes are minimal (and I am volunteering for these.)

Minimal as in "small diff" or "small effort"?

> 1. What is it?
> "A supercharged implementation of the [[https://github.com/mattgodbolt/compiler-explorer][godbolt compiler-explorer]] for Emacs.
>
> RMSbolt tries to make it easy to see what your compiler is doing. It does this
> by showing you the assembly output of a given source code file. It also
> highlights which source code a given assembly block corresponds to, and vice
> versa. It supports more types of languages than any previous tool of its kind.
> "

Sounds interesting.  I suppose the joke here is that RMS is God?

> It is currently hosted on gitlab and distributed on MELPA.

Could you provide a URL?

> 2. License
> From reading nongnu.elpa.git's README.org
>
> The package displays its License, which is Affero.
> Is the GNU Affero license considered suitable for NonGNU ELPA?

Yes, it should be.

>
> 3. Other
> I believe it follows the other requirements.
>
>
> WDYT?

-- 
	Philip Kaludercic on peregrine

Re: NonGNU ELPA: Conditions for accepting a potential new package 'rmsbolt' ?

[Jeremy Bryant]

Philip Kaludercic <philipk@posteo.net> writes:

> Jeremy Bryant <jb@jeremybryant.net> writes:
>
>> I would like to recommend the package 'rmsbolt' for NonGNU ELPA and
>> volunteer for any required changes.
>>
>> The author is Jay Kamat, who as I understand is potentially supportive
>> if the changes are minimal (and I am volunteering for these.)
>
> Minimal as in "small diff" or "small effort"?

I believe the author is supportive if the changes are a small effort.

>
>> 1. What is it?
>> "A supercharged implementation of the
>> [[https://github.com/mattgodbolt/compiler-explorer][godbolt
>> compiler-explorer]] for Emacs.
>>
>> RMSbolt tries to make it easy to see what your compiler is doing. It does this
>> by showing you the assembly output of a given source code file. It also
>> highlights which source code a given assembly block corresponds to, and vice
>> versa. It supports more types of languages than any previous tool of its kind.
>> "
>
> Sounds interesting.  I suppose the joke here is that RMS is God?

I believe Jay was inspired by https://godbolt.org/, authored by Matt
Godbolt

>
>> It is currently hosted on gitlab and distributed on MELPA.
>
> Could you provide a URL?

https://gitlab.com/jgkamat/rmsbolt

Re: NonGNU ELPA: Conditions for accepting a potential new package 'rmsbolt' ?

[Philip Kaludercic]

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

Jeremy Bryant <jb@jeremybryant.net> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> Jeremy Bryant <jb@jeremybryant.net> writes:
>>
>>> I would like to recommend the package 'rmsbolt' for NonGNU ELPA and
>>> volunteer for any required changes.
>>>
>>> The author is Jay Kamat, who as I understand is potentially supportive
>>> if the changes are minimal (and I am volunteering for these.)
>>
>> Minimal as in "small diff" or "small effort"?
>
> I believe the author is supportive if the changes are a small effort.

It would be nice if the checkdoc errors could be address, since there
are a few of them, but that is not a hard-constraint.

>>
>>> It is currently hosted on gitlab and distributed on MELPA.
>>
>> Could you provide a URL?
>
> https://gitlab.com/jgkamat/rmsbolt

Here are my comments:


[-- Attachment #2: Type: text/plain, Size: 32335 bytes --]

diff --git a/rmsbolt.el b/rmsbolt.el
index bdc457f..b2f2049 100644
--- a/rmsbolt.el
+++ b/rmsbolt.el
@@ -1,6 +1,7 @@
 ;;; rmsbolt.el --- A compiler output viewer -*- lexical-binding: t; -*-
 
-;; Copyright (C) 2018-2021 Jay Kamat
+;; Copyright (C) 2018-2023 Jay Kamat
+
 ;; Author: Jay Kamat <jaygkamat@gmail.com>
 ;; Version: 0.1.2
 ;; Keywords: compilation, tools
@@ -40,8 +41,10 @@
 ;; 4. Provide an interface for highlighting the matched assembly/bytecode line
 ;; to the source and vice versa
 ;;
-;; Tweakables:
-;; RMSBolt is primarily configured with Emacs local variables. This lets you
+
+;;;; Tweakables:
+
+;; RMSBolt is primarily configured with Emacs local variables.  This lets you
 ;; change compiler and rmsbolt options simply by editing a local variable block.
 ;;
 ;; Notable options:
@@ -80,25 +83,25 @@
 ;;; Code:
 ;;;; Customize:
 (defgroup rmsbolt nil
-  "rmsbolt customization options"
+  "rmsbolt customization options."
   :group 'applications)
 
 (defcustom rmsbolt-use-overlays t
   "Whether we should use overlays to show matching code."
-  :type 'boolean
-  :group 'rmsbolt)
+  :type 'boolean)
+
 (defcustom rmsbolt-goto-match t
   "Whether we should goto the match in the other buffer if it is non visible."
-  :type 'boolean
-  :group 'rmsbolt)
-(defcustom rmsbolt-mode-lighter " RMS🗲"
+  :type 'boolean)
+
+(defcustom rmsbolt-mode-lighter " RMS🗲"	;could the unicode charachter be removed?  it can unnecessarily resize the mode line.
   "Lighter displayed in mode line when function `rmsbolt-mode' is active."
-  :type 'string
-  :group 'rmsbolt)
+  :type 'string)
+
 (defcustom rmsbolt-large-buffer-size 500
   "Number of lines past which a buffer is considred large."
-  :type 'integer
-  :group 'rmsbolt)
+  :type 'natnum)
+
 (defcustom rmsbolt-automatic-recompile t
   "Whether to automatically save and recompile the source buffer.
 This setting is automatically disabled on large buffers, set to
@@ -107,29 +110,28 @@ manually saved, set to `on-save'."
   :type '(choice (const :tag "Off" nil)
                  (const :tag "On save" on-save)
                  (const :tag "On" t)
-                 (const :tag "Always" force))
-  :group 'rmsbolt)
+                 (const :tag "Always" force)))
 
 ;;;;; Buffer Local Tweakables
 (defcustom rmsbolt-disassemble nil
   "Whether we should disassemble an output binary."
   :type 'boolean
-  :safe 'booleanp
-  :group 'rmsbolt)
+  :safe 'booleanp)
+
 (defcustom rmsbolt-command nil
   "The base command to run rmsbolt from."
   :type 'string
   ;; nil means use default command
-  :safe (lambda (v) (or (booleanp v) (stringp v)))
-  :group 'rmsbolt)
+  :safe (lambda (v) (or (booleanp v) (stringp v))))
+
 (defcustom rmsbolt-default-directory nil
   "The default directory to compile from.
 This must be an absolute path if set.
 Some exporters (such as pony) may not work with this set."
   :type 'string
   ;; nil means use default command
-  :safe (lambda (v) (or (booleanp v) (stringp v)))
-  :group 'rmsbolt)
+  :safe (lambda (v) (or (booleanp v) (stringp v))))
+
 (define-obsolete-variable-alias 'rmsbolt-intel-x86
   'rmsbolt-asm-format "RMSBolt-0.2"
   "Sorry about not providing a proper migration for this variable.
@@ -142,6 +144,7 @@ tool defaults -> nil
 
 This means that if you had rmsbolt-intel-x86 set manually, you
 are now getting tool defaults.")
+
 (defcustom rmsbolt-asm-format "intel"
   "Which output assembly format to use.
 
@@ -155,45 +158,44 @@ If you are not on x86, you most likely want to set this to nil.
 Since this defaults to \"intel\", implementers must support this
 being set (at worst falling back to nil if passed \"intel\")."
   :type 'string
-  :safe (lambda (v) (or (booleanp v) (stringp v)))
-  :group 'rmsbolt)
+  :safe (lambda (v) (or (booleanp v) (stringp v))))
+
 (defcustom rmsbolt-filter-directives t
   "Whether to filter assembly directives."
   :type 'boolean
-  :safe 'booleanp
-  :group 'rmsbolt)
+  :safe 'booleanp)
+
 (defcustom rmsbolt-filter-labels t
   "Whether to filter unused labels."
   :type 'boolean
-  :safe 'booleanp
-  :group 'rmsbolt)
+  :safe 'booleanp)
+
 (defcustom rmsbolt-filter-comment-only t
   "Whether to filter comment-only lines."
   :type 'boolean
-  :safe 'booleanp
-  :group 'rmsbolt)
+  :safe 'booleanp)
+
 (defcustom rmsbolt-ignore-binary-limit nil
-  "Whether to ignore the binary limit. Could hang emacs..."
+  "Whether to ignore the binary limit.  Could hang emacs..."
   :type 'boolean
-  :safe 'booleanp
-  :group 'rmsbolt)
+  :safe 'booleanp)
+
 (defcustom rmsbolt-demangle t
   "Whether to attempt to demangle the resulting assembly."
   :type 'boolean
-  :safe 'booleanp
-  :group 'rmsbolt)
+  :safe 'booleanp)
+
 (defcustom rmsbolt-flag-quirks t
   "Whether to tweak flags to enable as many features as possible.
 
 In most cases, we will try to honor flags in rmsbolt-command as
-much as possible. However, some features may be disabled with
-some odd combinations of flags. This variable controls
+much as possible.  However, some features may be disabled with
+some odd combinations of flags.  This variable controls
 removing/adding flags to handle those cases.
 
 Note that basic flags to ensure basic usage are always modified."
   :type 'boolean
-  :safe 'booleanp
-  :group 'rmsbolt)
+  :safe 'booleanp)
 
 (defcustom rmsbolt-after-parse-hook nil
   "Hook after all parsing is done, but before compile command is run.
@@ -201,7 +203,6 @@ Note that basic flags to ensure basic usage are always modified."
 Exercise caution when setting variables in this hook - doing so
 can disrupt rmsbolt state and cause issues. Variables set here
 may not be cleared to default as variables are usually."
-  :group 'rmsbolt
   :type 'hook)
 
 ;;;; Faces
@@ -241,16 +242,15 @@ Used to work around inconsistencies in alternative shells.")
 Please DO NOT modify this blindly, as this directory will get
 deleted on Emacs exit.")
 
-(defvar rmsbolt-dir nil
+(defvar rmsbolt-dir (and load-file-name (file-name-directory load-file-name))
   "The directory which rmsbolt is installed to.")
-(when load-file-name
-  (setq rmsbolt-dir (file-name-directory load-file-name)))
 
 (defvar-local rmsbolt-src-buffer nil)
 
 (defvar-local rmsbolt--real-src-file nil
   "If set, the real filename that we compiled from,
-probably due to a copy from this file.")
+probably due to a copy from this file.") ;^there is a checkdoc warning here
+
 ;; FIXME should we be unbinding the list here, or is setting nil good enough.
 (defvar-local rmsbolt--default-variables nil
   "A list of the buffer-local variables we filled in with defaults.
@@ -395,7 +395,7 @@ Please be careful when setting this, as it bypasses most logic and is
 generally not useful."))
 
 ;;;; Helper Functions
-(defun rmsbolt--convert-file-name-to-system-type (file-name)
+(defun rmsbolt--convert-file-name-to-system-type (file-name) ;perhaps use `convert-standard-filename'?
   "Convert the argument FILE-NAME to windows format if `system-type' is equal to `cygwin'.
 Additional escaping with double quotes included to avoid backslashes loss in cygwin environment.
 If not `cygwin' then bypass the FILE-NAME."
@@ -417,7 +417,7 @@ Return value is quoted for passing to the shell."
             (rmsbolt-output-filename ,src-buffer)))))
      ,@body))
 
-(defmacro rmsbolt--set-local (var val)
+(defmacro rmsbolt--set-local (var val)	;`setq-local' is avaliable from 24.3 onwards
   "Set unquoted variable VAR to value VAL in current buffer."
   (declare (debug (symbolp form)))
   `(set (make-local-variable ,var) ,val))
@@ -513,6 +513,7 @@ this."
                                            " "))
                           " ")))
      cmd)))
+
 (cl-defun rmsbolt--lisp-compile-cmd (&key src-buffer)
   "Process a compile command for common lisp.
 
@@ -542,6 +543,7 @@ this."
                    " "))
        (_
         (error "This Common Lisp interpreter is not supported"))))))
+
 (cl-defun rmsbolt--rust-compile-cmd (&key src-buffer)
   "Process a compile command for rustc."
   (rmsbolt--with-files
@@ -563,6 +565,7 @@ this."
                                   (concat "-Cllvm-args=--x86-asm-syntax=" asm-format)))
                           " ")))
      cmd)))
+
 (cl-defun rmsbolt--go-compile-cmd (&key src-buffer)
   "Process a compile command for go."
   (rmsbolt--with-files
@@ -576,12 +579,13 @@ this."
                                 src-filename)
                           " ")))
      cmd)))
+
 (cl-defun rmsbolt--d-compile-cmd (&key src-buffer)
   "Process a compile command for d"
   (rmsbolt--with-files
    src-buffer
    (let* ((compiler (buffer-local-value 'rmsbolt-command src-buffer))
-          (cmd (mapconcat
+          (cmd (mapconcat		;you have `string-join' in subr-x
                 #'identity
                 (list compiler "-g" "-output-s" src-filename "-of" output-filename)
                 " ")))
@@ -624,6 +628,7 @@ this."
                                (buffer-file-name))
                               dir)))
     cmd))
+
 (cl-defun rmsbolt--py-compile-cmd (&key src-buffer)
   "Process a compile command for python3."
   (rmsbolt--with-files
@@ -676,6 +681,7 @@ https://github.com/derickr/vld"
                                 "-o" output-filename)
                           " ")))
      cmd)))
+
 (cl-defun rmsbolt--java-compile-cmd (&key src-buffer)
   "Process a compile command for ocaml.
 
@@ -771,6 +777,7 @@ https://github.com/derickr/vld"
               "frame_dummy"
               (and ".plt" (0+ any)))
       eol))
+
 (defvar rmsbolt--hidden-func-ocaml
   (rx bol
       (or (and "__" (0+ any))
@@ -782,11 +789,13 @@ https://github.com/derickr/vld"
           (and (or "caml_" "camlStd_") (0+ any))
           (and "caml" (or "Pervasives" "List" "Bytes"
                           "String" "Buffer" "Printf"
-                          "Char" "Sys") "__" (0+ any))
+                          "Char" "Sys")
+	       "__" (0+ any))
           ;; Ocaml likes to make labels following camlModule__,
           ;; filter out any lowercase
           (and (1+ (1+ lower) (opt (or "64" "32" "8" "16")) (opt "_"))))
       eol))
+
 (defvar rmsbolt--hidden-func-zig
   (rx bol (or (and "_" (0+ any))
               (and (opt "de") "register_tm_clones")
@@ -810,8 +819,7 @@ https://github.com/derickr/vld"
   (rmsbolt--path-to-swift-tool "swiftc"))
 
 (defun rmsbolt--path-to-swift-tool (swift-tool)
-  "Return the path to SWIFT-TOOL, depending on the active
-toolchain."
+  "Return the path to SWIFT-TOOL, depending on the active toolchain."
   (let* ((swift-tool-binary swift-tool)
          (swift-tool-toolchain-path (shell-command-to-string (format "echo -n `xcrun --find %s`" swift-tool-binary))))
     ;; If we have the Swift tool in PATH, just return it (this is the
@@ -821,8 +829,7 @@ toolchain."
      ((executable-find swift-tool-binary)
       swift-tool-binary)
      ((executable-find swift-tool-toolchain-path)
-      swift-tool-toolchain-path)
-     (t nil))))
+      swift-tool-toolchain-path))))
 
 (defun rmsbolt--parse-compile-commands (comp-cmds file)
   "Parse COMP-CMDS and extract a compilation dir and command for FILE."
@@ -840,6 +847,7 @@ toolchain."
              (dir (alist-get 'directory entry))
              (cmd (alist-get 'command entry)))
     (list dir cmd)))
+
 (defun rmsbolt--handle-c-compile-cmd (src-buffer)
   "Handle compile_commands.json for c/c++ for a given SRC-BUFFER.
 return t if successful."
@@ -866,126 +874,125 @@ return t if successful."
                     (rmsbolt-split-rm-single "-flto" #'string-prefix-p)
                     (rmsbolt-split-rm-double "-o")))
       t)))
+
 ;;;; Language Definitions
-(defvar rmsbolt-languages)
-(setq
- rmsbolt-languages
- `((c-mode
-    . ,(make-rmsbolt-lang :compile-cmd "gcc"
-                          :supports-asm t
-                          :supports-disass t
-                          :demangler "c++filt"
-                          :compile-cmd-function #'rmsbolt--c-compile-cmd
-                          :disass-hidden-funcs rmsbolt--hidden-func-c))
-   (c++-mode
-    . ,(make-rmsbolt-lang :compile-cmd "g++"
-                          :supports-asm t
-                          :supports-disass t
-                          :demangler "c++filt"
-                          :compile-cmd-function #'rmsbolt--c-compile-cmd
-                          :disass-hidden-funcs rmsbolt--hidden-func-c))
-   (d-mode
-    . ,(make-rmsbolt-lang :compile-cmd "ldc2"
-                          :supports-asm t
-                          :supports-disass nil
-                          :demangler "ddemangle"
-                          :compile-cmd-function #'rmsbolt--d-compile-cmd))
-   ;; In order to parse ocaml files, you need the emacs ocaml mode, tuareg
-   (tuareg-mode
-    . ,(make-rmsbolt-lang :compile-cmd "ocamlopt"
-                          :supports-asm t
-                          :supports-disass t
-                          :compile-cmd-function #'rmsbolt--ocaml-compile-cmd
-                          :disass-hidden-funcs rmsbolt--hidden-func-ocaml))
-   (lisp-mode
-    . ,(make-rmsbolt-lang :compile-cmd "sbcl"
-                          :supports-asm t
-                          :supports-disass nil
-                          :objdumper 'cat
-                          :compile-cmd-function #'rmsbolt--lisp-compile-cmd))
-   (rust-mode
-    . ,(make-rmsbolt-lang :compile-cmd "rustc"
-                          :supports-asm t
-                          :supports-disass nil
-                          :objdumper 'objdump
-                          :demangler "rustfilt"
-                          :compile-cmd-function #'rmsbolt--rust-compile-cmd))
-   ;; Copy of above
-   (rustic-mode
-    . ,(make-rmsbolt-lang :compile-cmd "rustc"
-                          :supports-asm t
-                          :supports-disass nil
-                          :objdumper 'objdump
-                          :demangler "rustfilt"
-                          :compile-cmd-function #'rmsbolt--rust-compile-cmd))
-   (ponylang-mode
-    . ,(make-rmsbolt-lang :compile-cmd "ponyc"
-                          :supports-asm t
-                          :supports-disass t
-                          :objdumper 'objdump
-                          :compile-cmd-function #'rmsbolt--pony-compile-cmd))
-   (php-mode
-    . ,(make-rmsbolt-lang :compile-cmd #'rmsbolt--php-default-compile-cmd
-                          :supports-asm t
-                          :supports-disass nil
-                          :compile-cmd-function #'rmsbolt--php-compile-cmd
-                          :process-asm-custom-fn #'rmsbolt--process-php-bytecode))
-   ;; ONLY SUPPORTS PYTHON 3
-   (python-mode
-    . ,(make-rmsbolt-lang :compile-cmd "python3"
-                          :supports-asm t
-                          :supports-disass nil
-                          :compile-cmd-function #'rmsbolt--py-compile-cmd
-                          :process-asm-custom-fn #'rmsbolt--process-python-bytecode))
-   (haskell-mode
-    . ,(make-rmsbolt-lang :compile-cmd "ghc"
-                          :supports-asm t
-                          :supports-disass nil
-                          :demangler "haskell-demangler"
-                          :compile-cmd-function #'rmsbolt--hs-compile-cmd))
-   (java-mode
-    . ,(make-rmsbolt-lang :compile-cmd "javac"
-                          :supports-asm t
-                          :supports-disass nil
-                          :objdumper 'cat
-                          :compile-cmd-function #'rmsbolt--java-compile-cmd
-                          :process-asm-custom-fn #'rmsbolt--process-java-bytecode))
-   (emacs-lisp-mode
-    . ,(make-rmsbolt-lang :supports-asm t
-                          :supports-disass nil
-                          ;; Nop
-                          :process-asm-custom-fn (lambda (_src-buffer lines)
-                                                   lines)
-                          :elisp-compile-override #'rmsbolt--elisp-compile-override))
-   (nim-mode
-    . ,(make-rmsbolt-lang :compile-cmd "nim c"
-                          :supports-disass t
-                          :objdumper 'objdump
-                          :demangler "c++filt"
-                          :compile-cmd-function #'rmsbolt--nim-compile-cmd
-                          :disass-hidden-funcs rmsbolt--hidden-func-c))
-   (zig-mode
-    . ,(make-rmsbolt-lang :compile-cmd "zig build-obj -O ReleaseFast"
-                          :supports-asm t
-                          :supports-disass t
-                          :objdumper 'objdump
-                          :compile-cmd-function #'rmsbolt--zig-compile-cmd
-                          :disass-hidden-funcs rmsbolt--hidden-func-zig))
-   (go-mode
-    . ,(make-rmsbolt-lang :compile-cmd "go"
-			                    :supports-asm nil
-			                    :supports-disass t
-			                    :objdumper 'go-objdump
-			                    :compile-cmd-function #'rmsbolt--go-compile-cmd
-			                    :process-asm-custom-fn #'rmsbolt--process-go-asm-lines))
-   (swift-mode
-    . ,(make-rmsbolt-lang :compile-cmd (rmsbolt--path-to-swift-compiler)
-                          :supports-asm t
-                          :supports-disass nil
-                          :objdumper 'objdump
-                          :demangler (rmsbolt--path-to-swift-demangler)
-                          :compile-cmd-function #'rmsbolt--swift-compile-cmd))
-   ))
+(defvar rmsbolt-languages
+  `((c-mode
+     . ,(make-rmsbolt-lang :compile-cmd "gcc"
+                           :supports-asm t
+                           :supports-disass t
+                           :demangler "c++filt"
+                           :compile-cmd-function #'rmsbolt--c-compile-cmd
+                           :disass-hidden-funcs rmsbolt--hidden-func-c))
+    (c++-mode
+     . ,(make-rmsbolt-lang :compile-cmd "g++"
+                           :supports-asm t
+                           :supports-disass t
+                           :demangler "c++filt"
+                           :compile-cmd-function #'rmsbolt--c-compile-cmd
+                           :disass-hidden-funcs rmsbolt--hidden-func-c))
+    (d-mode
+     . ,(make-rmsbolt-lang :compile-cmd "ldc2"
+                           :supports-asm t
+                           :supports-disass nil
+                           :demangler "ddemangle"
+                           :compile-cmd-function #'rmsbolt--d-compile-cmd))
+    ;; In order to parse ocaml files, you need the emacs ocaml mode, tuareg
+    (tuareg-mode
+     . ,(make-rmsbolt-lang :compile-cmd "ocamlopt"
+                           :supports-asm t
+                           :supports-disass t
+                           :compile-cmd-function #'rmsbolt--ocaml-compile-cmd
+                           :disass-hidden-funcs rmsbolt--hidden-func-ocaml))
+    (lisp-mode
+     . ,(make-rmsbolt-lang :compile-cmd "sbcl"
+                           :supports-asm t
+                           :supports-disass nil
+                           :objdumper 'cat
+                           :compile-cmd-function #'rmsbolt--lisp-compile-cmd))
+    (rust-mode
+     . ,(make-rmsbolt-lang :compile-cmd "rustc"
+                           :supports-asm t
+                           :supports-disass nil
+                           :objdumper 'objdump
+                           :demangler "rustfilt"
+                           :compile-cmd-function #'rmsbolt--rust-compile-cmd))
+    ;; Copy of above
+    (rustic-mode
+     . ,(make-rmsbolt-lang :compile-cmd "rustc"
+                           :supports-asm t
+                           :supports-disass nil
+                           :objdumper 'objdump
+                           :demangler "rustfilt"
+                           :compile-cmd-function #'rmsbolt--rust-compile-cmd))
+    (ponylang-mode
+     . ,(make-rmsbolt-lang :compile-cmd "ponyc"
+                           :supports-asm t
+                           :supports-disass t
+                           :objdumper 'objdump
+                           :compile-cmd-function #'rmsbolt--pony-compile-cmd))
+    (php-mode
+     . ,(make-rmsbolt-lang :compile-cmd #'rmsbolt--php-default-compile-cmd
+                           :supports-asm t
+                           :supports-disass nil
+                           :compile-cmd-function #'rmsbolt--php-compile-cmd
+                           :process-asm-custom-fn #'rmsbolt--process-php-bytecode))
+    ;; ONLY SUPPORTS PYTHON 3
+    (python-mode
+     . ,(make-rmsbolt-lang :compile-cmd "python3"
+                           :supports-asm t
+                           :supports-disass nil
+                           :compile-cmd-function #'rmsbolt--py-compile-cmd
+                           :process-asm-custom-fn #'rmsbolt--process-python-bytecode))
+    (haskell-mode
+     . ,(make-rmsbolt-lang :compile-cmd "ghc"
+                           :supports-asm t
+                           :supports-disass nil
+                           :demangler "haskell-demangler"
+                           :compile-cmd-function #'rmsbolt--hs-compile-cmd))
+    (java-mode
+     . ,(make-rmsbolt-lang :compile-cmd "javac"
+                           :supports-asm t
+                           :supports-disass nil
+                           :objdumper 'cat
+                           :compile-cmd-function #'rmsbolt--java-compile-cmd
+                           :process-asm-custom-fn #'rmsbolt--process-java-bytecode))
+    (emacs-lisp-mode
+     . ,(make-rmsbolt-lang :supports-asm t
+                           :supports-disass nil
+                           ;; Nop
+                           :process-asm-custom-fn (lambda (_src-buffer lines)
+                                                    lines)
+                           :elisp-compile-override #'rmsbolt--elisp-compile-override))
+    (nim-mode
+     . ,(make-rmsbolt-lang :compile-cmd "nim c"
+                           :supports-disass t
+                           :objdumper 'objdump
+                           :demangler "c++filt"
+                           :compile-cmd-function #'rmsbolt--nim-compile-cmd
+                           :disass-hidden-funcs rmsbolt--hidden-func-c))
+    (zig-mode
+     . ,(make-rmsbolt-lang :compile-cmd "zig build-obj -O ReleaseFast"
+                           :supports-asm t
+                           :supports-disass t
+                           :objdumper 'objdump
+                           :compile-cmd-function #'rmsbolt--zig-compile-cmd
+                           :disass-hidden-funcs rmsbolt--hidden-func-zig))
+    (go-mode
+     . ,(make-rmsbolt-lang :compile-cmd "go"
+			   :supports-asm nil
+			   :supports-disass t
+			   :objdumper 'go-objdump
+			   :compile-cmd-function #'rmsbolt--go-compile-cmd
+			   :process-asm-custom-fn #'rmsbolt--process-go-asm-lines))
+    (swift-mode
+     . ,(make-rmsbolt-lang :compile-cmd (rmsbolt--path-to-swift-compiler)
+                           :supports-asm t
+                           :supports-disass nil
+                           :objdumper 'objdump
+                           :demangler (rmsbolt--path-to-swift-demangler)
+                           :compile-cmd-function #'rmsbolt--swift-compile-cmd))
+    ))
 
 (defvar rmsbolt-c-dwarf-language
   (make-rmsbolt-lang :compile-cmd "gcc"
@@ -1014,7 +1021,6 @@ This should be an object of type `rmsbolt-lang', normally set by the major mode"
             display-buffer-overriding-action)))
      ,@body))
 
-
 ;;;; Functions
 ;; Functions to parse and lint assembly were lifted almost directly from the compiler-explorer
 
@@ -1066,9 +1072,7 @@ Lifted from https://emacs.stackexchange.com/questions/35936/disassembly-of-a-byt
   "Check if LINE has opcodes."
   (save-match-data
     (let* ((match (string-match rmsbolt-label-def line))
-           (line (if match
-                     (substring line (match-end 0))
-                   line))
+           (line (substring line (and match (match-end 0))))
            (line (cl-first (split-string line (rx (1+ (any ";#")))))))
       (if (string-match-p rmsbolt-assignment-def line)
           nil
@@ -1434,7 +1438,7 @@ Argument ASM-LINES input lines."
     (setq result (nconc die result))
     (nreverse result)))
 
-;;;;; Handlers
+;;;;; HANDLERS
 (cl-defun rmsbolt--handle-finish-compile (buffer str &key override-buffer stopped)
   "Finish hook for compilations.
 Argument BUFFER compilation buffer.
@@ -1476,23 +1480,22 @@ Argument STOPPED The compilation was stopped to start another compilation."
                    (let ((property
                           (get-text-property
                            0 'rmsbolt-src-line line)))
-                     (progn
-                       (cl-tagbody
-                        run-conditional
-                        (cond
-                         ((and in-match (eq in-match property))
-                          ;; We are continuing an existing match
-                          nil)
-                         (in-match
-                          ;; We are in a match that has just expired
-                          (push (cons start-match (1- linum))
-                                (gethash in-match ht))
-                          (setq in-match nil
-                                start-match nil)
-                          (go run-conditional))
-                         (property
-                          (setq in-match property
-                                start-match linum))))))
+                     (cl-tagbody
+                      run-conditional
+                      (cond
+                       ((and in-match (eq in-match property))
+                        ;; We are continuing an existing match
+                        nil)
+                       (in-match
+                        ;; We are in a match that has just expired
+                        (push (cons start-match (1- linum))
+                              (gethash in-match ht))
+                        (setq in-match nil
+                              start-match nil)
+                        (go run-conditional))
+                       (property
+                        (setq in-match property
+                              start-match linum)))))
                    (cl-incf linum))
 
                  (with-current-buffer src-buffer
@@ -1544,7 +1547,7 @@ Argument STOPPED The compilation was stopped to start another compilation."
                  ;; https://github.com/renzmann/treesit-auto/blob/d32617b5edb660b8a046053af3b92cf14f9b978e/treesit-auto.el#L89
                  (assoc (thread-last
                           (symbol-name major-mode)
-                          (replace-regexp-in-string "ts-mode$" "mode")
+                          (replace-regexp-in-string "-ts-mode\\'" "-mode")
                           (intern))
                         rmsbolt-languages)))))
 
@@ -1608,6 +1611,7 @@ and return it."
 (defun rmsbolt-compile ()
   "Compile the current rmsbolt buffer."
   (interactive)
+  ;; perhaps use `save-some-buffers'?
   (when (and (buffer-modified-p)
              (yes-or-no-p (format "Save buffer %s? " (buffer-name))))
     (save-buffer))
@@ -1615,7 +1619,7 @@ and return it."
   ;; Current buffer = src-buffer at this point
   (setq rmsbolt-src-buffer (current-buffer))
   (cond
-   ((eq major-mode 'asm-mode)
+   ((derived-mode-p 'asm-mode)
     ;; We cannot compile asm-mode files
     (message "Cannot compile assembly files. Are you sure you are not in the output buffer?"))
    ((rmsbolt-l-elisp-compile-override (rmsbolt--get-lang))
@@ -1641,8 +1645,7 @@ and return it."
                                   rmsbolt--temp-dir)))
       (run-hooks 'rmsbolt-after-parse-hook)
       (when (buffer-local-value 'rmsbolt-disassemble src-buffer)
-        (pcase
-            (rmsbolt-l-objdumper lang)
+        (pcase-exhaustive (rmsbolt-l-objdumper lang)
           ('objdump
            (setq cmd
                  (mapconcat #'identity
@@ -1670,9 +1673,7 @@ and return it."
                                   "&&" "mv"
                                   (rmsbolt-output-filename src-buffer)
                                   (rmsbolt-output-filename src-buffer t))
-                            " ")))
-          (_
-           (error "Objdumper not recognized"))))
+                            " ")))))
       ;; Convert to demangle if we need to
       (setq cmd (rmsbolt--demangle-command cmd lang src-buffer))
       (with-current-buffer ; With compilation buffer
@@ -1720,14 +1721,14 @@ and return it."
                            rmsbolt--temp-dir
                            (file-directory-p rmsbolt--temp-dir))
                   (delete-directory rmsbolt--temp-dir t))
-                (setq rmsbolt--temp-dir nil)))))
+                (setq rmsbolt--temp-dir nil))))) ;why is this necessary while killing emacs?
 
 ;;;;; Starter Definitions
 
 ;; IIUC, this "starter" business is not a necessary part of RMSBolt, but is
 ;; a way to provide sample files with which users can try out RMSBolt.
 
-(defvar rmsbolt-starter-files
+(defvar rmsbolt-starter-files		;should this be part of a secondary file that isn't loaded by default?
   '(("c" . "rmsbolt.c")
     ("c++" . "rmsbolt.cpp")
     ("ocaml" . "rmsbolt.ml")
@@ -1778,24 +1779,23 @@ and return it."
   (when line
     (let ((cur (line-number-at-pos)))
       (forward-line (- line cur)))))
+
 (defun rmsbolt--setup-overlay (start end buf)
   "Setup overlay with START and END in BUF."
   (let ((o (make-overlay start end buf)))
     (overlay-put o 'face 'rmsbolt-current-line-face)
     o))
+
 (cl-defun rmsbolt--point-visible (point)
   "Check if the current point is visible in a window in the current buffer."
-  (when (cl-find-if (lambda (w)
-                      (and (>= point (window-start w))
-                           (<= point (window-end w))))
-                    (get-buffer-window-list))
-    t))
+  (cl-find-if (lambda (w)
+		(<= (window-start w) point (window-end w)))
+              (get-buffer-window-list)))
 
 (cl-defun rmsbolt-update-overlays (&key (force nil))
   "Update overlays to highlight the currently selected source and asm lines.
-  If FORCE, always scroll overlay, even when one is visible.
-  FORCE also scrolls to the first line, instead of the first line
-  of the last block."
+If FORCE, always scroll overlay, even when one is visible.  FORCE also
+scrolls to the first line, instead of the first line of the last block."
   (when rmsbolt-mode
     (if-let ((should-run rmsbolt-use-overlays)
              (output-buffer (get-buffer rmsbolt-output-buffer))
@@ -1841,8 +1841,7 @@ and return it."
                                          (not scroll-src-buffer-p))
                                 (setq line-visible (or (rmsbolt--point-visible start-pt)
                                                        (rmsbolt--point-visible end-pt)
-                                                       (and (> saved-pt start-pt)
-                                                            (< saved-pt end-pt)))))
+                                                       (< start-pt saved-pt end-pt))))
                               (push (rmsbolt--setup-overlay start-pt end-pt output-buffer)
                                     rmsbolt-overlays)))))
             (when (or (not line-visible) force)
@@ -1931,9 +1930,7 @@ and return it."
   "Toggle rmsbolt-mode.
 
 This mode is enabled in both src and assembly output buffers."
-  :global nil
   :lighter rmsbolt-mode-lighter
-  :keymap rmsbolt-mode-map
   ;; Init
   (cond
    (rmsbolt-mode

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



-- 
	Philip Kaludercic on peregrine

Re: NonGNU ELPA: Conditions for accepting a potential new package 'rmsbolt' ?

[Jay Kamat]

Philip Kaludercic <philipk@posteo.net> writes:

Might take me a while to fully review and test all the changes, but they look 
good, so once I do so, I'll include them!

> +(defcustom rmsbolt-mode-lighter " RMS🗲"	;could the unicode charachter be 
> removed?  it can unnecessarily resize the mode line.

Sure - I can remove it. I'll do it after I apply your patch generally.

> -(defun rmsbolt--convert-file-name-to-system-type (file-name)
> +(defun rmsbolt--convert-file-name-to-system-type (file-name) ;perhaps use 
> `convert-standard-filename'?

Right now, a contributor is trying to improve this area/enable tramp support - 
so I'd like to keep this as stable as possible while they do so. But this is a 
great suggestion - I'll let them know about this function and hopefully it 
will help them in their change as well!

> -(defmacro rmsbolt--set-local (var val)
> +(defmacro rmsbolt--set-local (var val)	;`setq-local' is avaliable from 
> 24.3 onwards

I'll make this change after I get this in, thanks!

> -          (cmd (mapconcat
> +          (cmd (mapconcat		;you have `string-join' in subr-x
>                  #'identity
>                  (list compiler "-g" "-output-s" src-filename "-of" 
>                  output-filename)
>                  " ")))

Not sure why I missed this - I'll fix this after this patch as well - thanks!

> -                (setq rmsbolt--temp-dir nil)))))
> +                (setq rmsbolt--temp-dir nil))))) ;why is this necessary 
> while killing emacs?

This is mostly me being very paranoid - as this function could delete random 
directories if misconfigured/in error, I want to disable it as soon as 
possible.

>
>  ;;;;; Starter Definitions
>
>  ;; IIUC, this "starter" business is not a necessary part of RMSBolt, but is
>  ;; a way to provide sample files with which users can try out RMSBolt.
>
> -(defvar rmsbolt-starter-files
> +(defvar rmsbolt-starter-files		;should this be part of a secondary 
> file that isn't loaded by default?

Maybe - I think I would load it by default though - as this is the easiest way 
for someone to try out the package. In my personal usage I almost always start 
from one of these files and add my changes. I should update the comment above 
with that information as well.

Thanks! I just want to be doubly sure of all the changes before I commit them, 
I'll try to do that testing tomorrow - I'm a bit busy today. I'll also try to 
fix any remaining checkdoc errors.

-Jay

Re: NonGNU ELPA: Conditions for accepting a potential new package 'rmsbolt' ?

[Philip Kaludercic]

Jay Kamat <jaygkamat@gmail.com> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
> Might take me a while to fully review and test all the changes, but
> they look good, so once I do so, I'll include them!
>
>> +(defcustom rmsbolt-mode-lighter " RMS🗲"	;could the unicode
>> charachter be removed?  it can unnecessarily resize the mode line.
>
> Sure - I can remove it. I'll do it after I apply your patch generally.

Just keep in mind that I didn't send a patch.  I just find it convenient
to send comments in form of a diff.

>> -(defun rmsbolt--convert-file-name-to-system-type (file-name)
>> +(defun rmsbolt--convert-file-name-to-system-type (file-name)
>> ;perhaps use `convert-standard-filename'?
>
> Right now, a contributor is trying to improve this area/enable tramp
> support - so I'd like to keep this as stable as possible while they do
> so. But this is a great suggestion - I'll let them know about this
> function and hopefully it will help them in their change as well!

No problem.

>>
>>  ;;;;; Starter Definitions
>>
>>  ;; IIUC, this "starter" business is not a necessary part of RMSBolt, but is
>>  ;; a way to provide sample files with which users can try out RMSBolt.
>>
>> -(defvar rmsbolt-starter-files
>> +(defvar rmsbolt-starter-files		;should this be part
>> of a secondary file that isn't loaded by default?
>
> Maybe - I think I would load it by default though - as this is the
> easiest way for someone to try out the package. In my personal usage I
> almost always start from one of these files and add my changes. I
> should update the comment above with that information as well.

It shouldn't matter, as long as the command that uses
`rmsbolt-starter-files' remains auto-loaded, even if the command is
not defined in the main file.

> Thanks! I just want to be doubly sure of all the changes before I
> commit them, I'll try to do that testing tomorrow - I'm a bit busy
> today. I'll also try to fix any remaining checkdoc errors.

Thanks!

> -Jay
>

-- 
	Philip Kaludercic on peregrine

Re: NonGNU ELPA: Conditions for accepting a potential new package 'rmsbolt' ?

[Jay Kamat]

Philip Kaludercic <philipk@posteo.net> writes:

> Jay Kamat <jaygkamat@gmail.com> writes:
>
>> Philip Kaludercic <philipk@posteo.net> writes:
>>
>> Might take me a while to fully review and test all the changes, but
>> they look good, so once I do so, I'll include them!
>>
>>> +(defcustom rmsbolt-mode-lighter " RMS🗲"	;could the unicode
>>> charachter be removed?  it can unnecessarily resize the mode line.
>>
>> Sure - I can remove it. I'll do it after I apply your patch generally.
>
> Just keep in mind that I didn't send a patch.  I just find it convenient
> to send comments in form of a diff.

Ah - I didn't realize. I think most of your code-changes are good, is it 
alright if I apply them? I'll make a commit with you as the author (or I can 
take authorship/not commit them - whatever your preference is).

>>>
>>>  ;;;;; Starter Definitions
>>>
>>>  ;; IIUC, this "starter" business is not a necessary part of RMSBolt, but 
>>>  is
>>>  ;; a way to provide sample files with which users can try out RMSBolt.
>>>
>>> -(defvar rmsbolt-starter-files
>>> +(defvar rmsbolt-starter-files		;should this be part
>>> of a secondary file that isn't loaded by default?
>>
>> Maybe - I think I would load it by default though - as this is the
>> easiest way for someone to try out the package. In my personal usage I
>> almost always start from one of these files and add my changes. I
>> should update the comment above with that information as well.
>
> It shouldn't matter, as long as the command that uses
> `rmsbolt-starter-files' remains auto-loaded, even if the command is
> not defined in the main file.

Ah - that's true! I'll try to make this change after the others if I have 
time.


Thanks again for taking a look!
-Jay

Re: NonGNU ELPA: Conditions for accepting a potential new package 'rmsbolt' ?

[Stefan Monnier]

>> -(defun rmsbolt--convert-file-name-to-system-type (file-name)
>> +(defun rmsbolt--convert-file-name-to-system-type (file-name) ;perhaps use
>> `convert-standard-filename'?
>
> Right now, a contributor is trying to improve this area/enable tramp
> support - so I'd like to keep this as stable as possible while they do
> so.  But this is a great suggestion - I'll let them know about this function
> and hopefully it will help them in their change as well!

I haven't looked at the code but I'll point out that
`convert-standard-filename` is meant to *define* the name of a "standard"
file, so its argument is usually very static.
It doesn't make sense to apply it to something like `buffer-file-name`
which is already supposed to be a valid filename.

Also, I see:

          (rmsbolt--convert-file-name-to-system-type
           (shell-quote-argument

which is very weird.  The reverse would make more sense.

Also I think `shell-quote-argument` should be applied to most parts of
`rmsbolt--demangle-command`.  This function is also odd in that it
calls `rmsbolt--convert-file-name-to-system-type` on the files
of the demangler but not those of the `mv`.


        Stefan

Re: NonGNU ELPA: Conditions for accepting a potential new package 'rmsbolt' ?

[Philip Kaludercic]

Jay Kamat <jaygkamat@gmail.com> writes:

> Philip Kaludercic <philipk@posteo.net> writes:
>
>> Jay Kamat <jaygkamat@gmail.com> writes:
>>
>>> Philip Kaludercic <philipk@posteo.net> writes:
>>>
>>> Might take me a while to fully review and test all the changes, but
>>> they look good, so once I do so, I'll include them!
>>>
>>>> +(defcustom rmsbolt-mode-lighter " RMS🗲"	;could the unicode
>>>> charachter be removed?  it can unnecessarily resize the mode line.
>>>
>>> Sure - I can remove it. I'll do it after I apply your patch generally.
>>
>> Just keep in mind that I didn't send a patch.  I just find it convenient
>> to send comments in form of a diff.
>
> Ah - I didn't realize. I think most of your code-changes are good, is
> it alright if I apply them? I'll make a commit with you as the author
> (or I can take authorship/not commit them - whatever your preference
> is).

Of course, I just wanted to make sure that you don't apply a comment
instead of addressing it.  Also, some changes might have been
opinionated (I don't recall), and I wouldn't want to insinuate that you
must apply them.

I don't care about authorship, do as you like.

>>>>
>>>>  ;;;;; Starter Definitions
>>>>
>>>>  ;; IIUC, this "starter" business is not a necessary part of
>>>> RMSBolt, but  is
>>>>  ;; a way to provide sample files with which users can try out RMSBolt.
>>>>
>>>> -(defvar rmsbolt-starter-files
>>>> +(defvar rmsbolt-starter-files		;should this be part
>>>> of a secondary file that isn't loaded by default?
>>>
>>> Maybe - I think I would load it by default though - as this is the
>>> easiest way for someone to try out the package. In my personal usage I
>>> almost always start from one of these files and add my changes. I
>>> should update the comment above with that information as well.
>>
>> It shouldn't matter, as long as the command that uses
>> `rmsbolt-starter-files' remains auto-loaded, even if the command is
>> not defined in the main file.
>
> Ah - that's true! I'll try to make this change after the others if I
> have time.

1+

-- 
	Philip Kaludercic on peregrine

Re: NonGNU ELPA: Conditions for accepting a potential new package 'rmsbolt' ?

[Philip Kaludercic]

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

>>> -(defun rmsbolt--convert-file-name-to-system-type (file-name)
>>> +(defun rmsbolt--convert-file-name-to-system-type (file-name) ;perhaps use
>>> `convert-standard-filename'?
>>
>> Right now, a contributor is trying to improve this area/enable tramp
>> support - so I'd like to keep this as stable as possible while they do
>> so.  But this is a great suggestion - I'll let them know about this function
>> and hopefully it will help them in their change as well!
>
> I haven't looked at the code but I'll point out that
> `convert-standard-filename` is meant to *define* the name of a "standard"
> file, so its argument is usually very static.
> It doesn't make sense to apply it to something like `buffer-file-name`
> which is already supposed to be a valid filename.

In that case I must have misunderstood something, my intention was to
find a function that would convert file names with forwards slashes to
the proper file delimiters on the current system.

> Also, I see:
>
>           (rmsbolt--convert-file-name-to-system-type
>            (shell-quote-argument
>
> which is very weird.  The reverse would make more sense.
>
> Also I think `shell-quote-argument` should be applied to most parts of
> `rmsbolt--demangle-command`.  This function is also odd in that it
> calls `rmsbolt--convert-file-name-to-system-type` on the files
> of the demangler but not those of the `mv`.

IIUC shell-quote-argument cannot be used, since rmsbolt is actually
creating a shell command out of a list, where each element of the list
is not necessarily a command line argument.  E.g. there were some places
where "&&" or ">" were being used, and quoting those would break the
intended functionality.  I have to admit that the process of creating
commands this way seems fragile, but there is a lot of code that would
take a while to rewrite and verify that it doesn't break anything.  (A
different idea would be to have a defstruct for each command that would be
prepared into a list and in the end executed in order, ideally using
some function like `process-file').

-- 
	Philip Kaludercic on peregrine

Re: NonGNU ELPA: Conditions for accepting a potential new package 'rmsbolt' ?

[Stefan Monnier]

>> I haven't looked at the code but I'll point out that
>> `convert-standard-filename` is meant to *define* the name of a "standard"
>> file, so its argument is usually very static.
>> It doesn't make sense to apply it to something like `buffer-file-name`
>> which is already supposed to be a valid filename.
>
> In that case I must have misunderstood something, my intention was to
> find a function that would convert file names with forwards slashes to
> the proper file delimiters on the current system.

No, `convert-standard-filename` is useful when you got some string from
somewhere and you'd like to generate a valid file name from that.
Usually the string is an almost constant like "~/.emacs" and the purpose
is to deal with quirks like when file names can't start with a dot or can't
include a colon.

I can't help much more on if/when/how to convert an internal filename
into one that works in a win32 shell, 'cause I have no experience with
that.  AFAICT most ELisp code doesn't do anything special for that
(whereas they sometimes do such things for Tramp purposes), so I'd
naively expect that it "just works".

>> Also I think `shell-quote-argument` should be applied to most parts of
>> `rmsbolt--demangle-command`.  This function is also odd in that it
>> calls `rmsbolt--convert-file-name-to-system-type` on the files
>> of the demangler but not those of the `mv`.
>
> IIUC shell-quote-argument cannot be used,

Of course it can.  You can't just replace the #'identity with
#'shell-quote-argument, indeed, but still `shell-quote-argument` should
be used on *most* parts that appear in the function.


        Stefan

Re: NonGNU ELPA: Conditions for accepting a potential new package 'rmsbolt' ?

[Philip Kaludercic]

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

> I can't help much more on if/when/how to convert an internal filename
> into one that works in a win32 shell, 'cause I have no experience with
> that.  AFAICT most ELisp code doesn't do anything special for that
> (whereas they sometimes do such things for Tramp purposes), so I'd
> naively expect that it "just works".

Inside of Emacs, but does it also handle argument when starting a
process with some path?

>>> Also I think `shell-quote-argument` should be applied to most parts of
>>> `rmsbolt--demangle-command`.  This function is also odd in that it
>>> calls `rmsbolt--convert-file-name-to-system-type` on the files
>>> of the demangler but not those of the `mv`.
>>
>> IIUC shell-quote-argument cannot be used,
>
> Of course it can.  You can't just replace the #'identity with
> #'shell-quote-argument, indeed, but still `shell-quote-argument` should
> be used on *most* parts that appear in the function.

True, my comment was not phrased carefully.

-- 
	Philip Kaludercic on peregrine

Re: NonGNU ELPA: Conditions for accepting a potential new package 'rmsbolt' ?

[Eli Zaretskii]

> From: Stefan Monnier <monnier@iro.umontreal.ca>
> Cc: Jay Kamat <jaygkamat@gmail.com>,  Jeremy Bryant <jb@jeremybryant.net>,
>  "emacs-devel@gnu.org" <emacs-devel@gnu.org>
> Date: Tue, 27 Feb 2024 12:49:51 -0500
> 
> No, `convert-standard-filename` is useful when you got some string from
> somewhere and you'd like to generate a valid file name from that.
> Usually the string is an almost constant like "~/.emacs" and the purpose
> is to deal with quirks like when file names can't start with a dot or can't
> include a colon.

Right.

> I can't help much more on if/when/how to convert an internal filename
> into one that works in a win32 shell, 'cause I have no experience with
> that.  AFAICT most ELisp code doesn't do anything special for that
> (whereas they sometimes do such things for Tramp purposes), so I'd
> naively expect that it "just works".

It "just works" because we do the necessary conversion and massage
under the hood.  Whether it will work in this case depends on what
Lisp APIs are used for that.

Re: NonGNU ELPA: Conditions for accepting a potential new package 'rmsbolt' ?

[Stefan Monnier]

>> I can't help much more on if/when/how to convert an internal filename
>> into one that works in a win32 shell, 'cause I have no experience with
>> that.  AFAICT most ELisp code doesn't do anything special for that
>> (whereas they sometimes do such things for Tramp purposes), so I'd
>> naively expect that it "just works".
> Inside of Emacs, but does it also handle argument when starting a
> process with some path?

The above paragraph is specifically talking about that (hence the "a
win32 shell") 🙂


        Stefan

Re: NonGNU ELPA: Conditions for accepting a potential new package 'rmsbolt' ?

[Jay Kamat]

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

>
> Also, I see:
>
>           (rmsbolt--convert-file-name-to-system-type
>            (shell-quote-argument
>
> which is very weird.  The reverse would make more sense.

I agree - this system-type function only does something on cygwin systems 
currently and I don't have any windows machines - so I'm unable to test it at 
all, which is why I avoided changing this. I might try to swap these around 
(and probably remove some escaping in the system-type function) and see if 
anyone complains.

> Also I think `shell-quote-argument` should be applied to most parts of
> `rmsbolt--demangle-command`.  This function is also odd in that it
> calls `rmsbolt--convert-file-name-to-system-type` on the files
> of the demangler but not those of the `mv`.

I agree - I've been a little lax about that, especially in this function - 
once I get some more time, I'll try to refactor things so there's one 
"filename quoting" function, that simply runs `shell-quote-argument' on unix 
and additionally does the cygwin conversion if needed - that way this logic is 
consistent.

Thanks so much for taking a look, regardless, it really helps improve the 
quality of the codebase!

-Jay