bug#6207: 23.1; align.el vhdl rules setup

bug#6207: 23.1; align.el vhdl rules setup

[Kevin Ryde]

align.el has some setups for vhdl-mode, but the way align.el adds to
vhdl-mode-hook means they only take effect in vhdl-mode buffers made
after align.el has loaded, whereas I hoped they would be always present.

Eg. the foo.vhd below

    emacs -Q foo.vhd
    M-x end-of-buffer
    M-x align

does nothing, but

    emacs -Q
    M-: (require 'align)
    C-x C-f foo.vhd
    M-x end-of-buffer
    M-x align

lines up the "signal" and "variable" expressions.

I see there's a comment in align.el that its vhdl stuff belongs in
vhdl-mode.el.  I suppose that would be the right way to do it.

(vhdl-mode has some of its own alignment, the align.el bits are separate
and don't interfere as far as I can tell.  From the comments I get the
impression align.el came afterwards, or some such.)


In GNU Emacs 23.1.1 (i486-pc-linux-gnu, GTK+ Version 2.16.5)
 of 2009-09-14 on raven, modified by Debian
configured using `configure  '--build=i486-linux-gnu' '--host=i486-linux-gnu' '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib' '--localstatedir=/var/lib' '--infodir=/usr/share/info' '--mandir=/usr/share/man' '--with-pop=yes' '--enable-locallisppath=/etc/emacs23:/etc/emacs:/usr/local/share/emacs/23.1/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/23.1/site-lisp:/usr/share/emacs/site-lisp:/usr/share/emacs/23.1/leim' '--with-x=yes' '--with-x-toolkit=gtk' '--with-toolkit-scroll-bars' 'build_alias=i486-linux-gnu' 'host_alias=i486-linux-gnu' 'CFLAGS=-DDEBIAN -g -O2' 'LDFLAGS=-g' 'CPPFLAGS=''

Important settings:
  value of $LC_ALL: nil
  value of $LC_COLLATE: nil
  value of $LC_CTYPE: nil
  value of $LC_MESSAGES: nil
  value of $LC_MONETARY: nil
  value of $LC_NUMERIC: nil
  value of $LC_TIME: nil
  value of $LANG: en_AU
  value of $XMODIFIERS: nil
  locale-coding-system: iso-latin-1-unix
  default-enable-multibyte-characters: t

bug#6207: 23.1; align.el vhdl rules setup

[Stefan Monnier]

Hi Reto,

What do you think of moving the VHDL settings of align.el to vhdl.el?
It sounds like The Right Thing to me,


        Stefan


> align.el has some setups for vhdl-mode, but the way align.el adds to
> vhdl-mode-hook means they only take effect in vhdl-mode buffers made
> after align.el has loaded, whereas I hoped they would be always present.

> Eg. the foo.vhd below

>     emacs -Q foo.vhd
>     M-x end-of-buffer
>     M-x align

> does nothing, but

>     emacs -Q
>     M-: (require 'align)
>     C-x C-f foo.vhd
>     M-x end-of-buffer
>     M-x align

> lines up the "signal" and "variable" expressions.

> I see there's a comment in align.el that its vhdl stuff belongs in
> vhdl-mode.el.  I suppose that would be the right way to do it.

> (vhdl-mode has some of its own alignment, the align.el bits are separate
> and don't interfere as far as I can tell.  From the comments I get the
> impression align.el came afterwards, or some such.)


> In GNU Emacs 23.1.1 (i486-pc-linux-gnu, GTK+ Version 2.16.5)
>  of 2009-09-14 on raven, modified by Debian
> configured using `configure  '--build=i486-linux-gnu' '--host=i486-linux-gnu' '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib' '--localstatedir=/var/lib' '--infodir=/usr/share/info' '--mandir=/usr/share/man' '--with-pop=yes' '--enable-locallisppath=/etc/emacs23:/etc/emacs:/usr/local/share/emacs/23.1/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/23.1/site-lisp:/usr/share/emacs/site-lisp:/usr/share/emacs/23.1/leim' '--with-x=yes' '--with-x-toolkit=gtk' '--with-toolkit-scroll-bars' 'build_alias=i486-linux-gnu' 'host_alias=i486-linux-gnu' 'CFLAGS=-DDEBIAN -g -O2' 'LDFLAGS=-g' 'CPPFLAGS=''

> Important settings:
>   value of $LC_ALL: nil
>   value of $LC_COLLATE: nil
>   value of $LC_CTYPE: nil
>   value of $LC_MESSAGES: nil
>   value of $LC_MONETARY: nil
>   value of $LC_NUMERIC: nil
>   value of $LC_TIME: nil
>   value of $LANG: en_AU
>   value of $XMODIFIERS: nil
>   locale-coding-system: iso-latin-1-unix
>   default-enable-multibyte-characters: t

bug#6207: 23.1; align.el vhdl rules setup

[John Wiegley]

On May 18, 2010, at 11:35 PM, Stefan Monnier wrote:

> What do you think of moving the VHDL settings of align.el to vhdl.el?
> It sounds like The Right Thing to me,

This sounds like the right thing to me too.  These settings were made in align.el long before it was as established as it is.

John

bug#6207: 23.1; align.el vhdl rules setup

[Reto Zimmermann]

Hi Stefan,

The settings in align.el are not used at all in vhdl-mode.  vhdl-mode has it's own alignment settings and commands, which have evolved since the settings in align.el were defined.  So the settings in align.el are obsolete and could be removed.

Reto


Stefan Monnier wrote:
> Hi Reto,
> 
> What do you think of moving the VHDL settings of align.el to vhdl.el?
> It sounds like The Right Thing to me,
> 
> 
>         Stefan
> 
> 
>> align.el has some setups for vhdl-mode, but the way align.el adds to
>> vhdl-mode-hook means they only take effect in vhdl-mode buffers made
>> after align.el has loaded, whereas I hoped they would be always present.
> 
>> Eg. the foo.vhd below
> 
>>     emacs -Q foo.vhd
>>     M-x end-of-buffer
>>     M-x align
> 
>> does nothing, but
> 
>>     emacs -Q
>>     M-: (require 'align)
>>     C-x C-f foo.vhd
>>     M-x end-of-buffer
>>     M-x align
> 
>> lines up the "signal" and "variable" expressions.
> 
>> I see there's a comment in align.el that its vhdl stuff belongs in
>> vhdl-mode.el.  I suppose that would be the right way to do it.
> 
>> (vhdl-mode has some of its own alignment, the align.el bits are separate
>> and don't interfere as far as I can tell.  From the comments I get the
>> impression align.el came afterwards, or some such.)
> 
> 
>> In GNU Emacs 23.1.1 (i486-pc-linux-gnu, GTK+ Version 2.16.5)
>>  of 2009-09-14 on raven, modified by Debian
>> configured using `configure  '--build=i486-linux-gnu' '--host=i486-linux-gnu' '--prefix=/usr' '--sharedstatedir=/var/lib' '--libexecdir=/usr/lib' '--localstatedir=/var/lib' '--infodir=/usr/share/info' '--mandir=/usr/share/man' '--with-pop=yes' '--enable-locallisppath=/etc/emacs23:/etc/emacs:/usr/local/share/emacs/23.1/site-lisp:/usr/local/share/emacs/site-lisp:/usr/share/emacs/23.1/site-lisp:/usr/share/emacs/site-lisp:/usr/share/emacs/23.1/leim' '--with-x=yes' '--with-x-toolkit=gtk' '--with-toolkit-scroll-bars' 'build_alias=i486-linux-gnu' 'host_alias=i486-linux-gnu' 'CFLAGS=-DDEBIAN -g -O2' 'LDFLAGS=-g' 'CPPFLAGS=''
> 
>> Important settings:
>>   value of $LC_ALL: nil
>>   value of $LC_COLLATE: nil
>>   value of $LC_CTYPE: nil
>>   value of $LC_MESSAGES: nil
>>   value of $LC_MONETARY: nil
>>   value of $LC_NUMERIC: nil
>>   value of $LC_TIME: nil
>>   value of $LANG: en_AU
>>   value of $XMODIFIERS: nil
>>   locale-coding-system: iso-latin-1-unix
>>   default-enable-multibyte-characters: t
> 
> 
> 

bug#6207: 23.1; align.el vhdl rules setup

[Kevin Ryde]

Reto Zimmermann <reto@gnu.org> writes:
>
> So the settings in align.el are obsolete and could be removed.

The mechanisms in align.el seem a bit more general.
Should vhdl be using it perhaps with updates to the rules list?

bug#6207: 23.1; align.el vhdl rules setup

[Reto Zimmermann]

Kevin Ryde wrote:
> Reto Zimmermann <reto@gnu.org> writes:
>> So the settings in align.el are obsolete and could be removed.
> 
> The mechanisms in align.el seem a bit more general.
> Should vhdl be using it perhaps with updates to the rules list?

The alignment code in vhdl-mode is customized and fine-tuned for VHDL in a a way that I think cannot be implemented with the more general align.el.

bug#6207: 23.1; align.el vhdl rules setup

[Stefan Kangas]

[-- Attachment #1.1: Type: text/plain, Size: 352 bytes --]

Reto Zimmermann <reto@gnu.org> writes:

> The settings in align.el are not used at all in vhdl-mode.  vhdl-mode
> has it's own alignment settings and commands, which have evolved since
> the settings in align.el were defined.  So the settings in align.el
> are obsolete and could be removed.

How about the attached patch?

Best regards,
Stefan Kangas

[-- Attachment #1.2: Type: text/html, Size: 462 bytes --]

[-- Attachment #2: 0001-Declare-unused-vhdl-code-in-align.el-obsolete.patch --]
[-- Type: text/x-patch, Size: 2009 bytes --]

From 1ca009f47fba725a77a1d809b7ef9bb40327722c Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Thu, 26 Sep 2019 12:31:37 +0200
Subject: [PATCH] Declare unused vhdl code in align.el obsolete

* lisp/align.el (align-vhdl-rules-list, align-set-vhdl-rules): Declare
obsolete.  (Bug#6207)
(align-dq-string-modes, align-open-comment-modes): Move vhdl-mode to
definition instead of adding it later.
---
 lisp/align.el | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/lisp/align.el b/lisp/align.el
index cd72d52df4..52aecaefb4 100644
--- a/lisp/align.el
+++ b/lisp/align.el
@@ -206,7 +206,7 @@ align-text-modes
 
 (defcustom align-dq-string-modes
   (append align-lisp-modes align-c++-modes align-perl-modes
-	  '(python-mode))
+	  '(python-mode 'vhdl-mode))
   "A list of modes where double quoted strings should be excluded."
   :type '(repeat symbol)
   :group 'align)
@@ -219,7 +219,7 @@ align-sq-string-modes
 
 (defcustom align-open-comment-modes
   (append align-lisp-modes align-c++-modes align-perl-modes
-	  '(python-mode makefile-mode))
+	  '(python-mode makefile-mode vhdl-mode))
   "A list of modes with a single-line comment syntax.
 These are comments as in Lisp, which have a beginning, but end with
 the line (i.e., `comment-end' is an empty string)."
@@ -842,18 +842,14 @@ align-vhdl-rules-list
   "Alignment rules for `vhdl-mode'.  See `align-rules-list' for more info."
   :type align-rules-list-type
   :group 'align)
-
 (put 'align-vhdl-rules-list 'risky-local-variable t)
+(make-obsolete-variable 'align-vhdl-rules-list "not used" "27.1")
 
 (defun align-set-vhdl-rules ()
   "Setup the `align-mode-rules-list' variable for `vhdl-mode'."
+  (define (obsolete nil "27.1"))
   (setq align-mode-rules-list align-vhdl-rules-list))
 
-(add-hook 'vhdl-mode-hook 'align-set-vhdl-rules)
-
-(add-to-list 'align-dq-string-modes 'vhdl-mode)
-(add-to-list 'align-open-comment-modes 'vhdl-mode)
-
 ;;; User Functions:
 
 ;;;###autoload
-- 
2.20.1

bug#6207: 23.1; align.el vhdl rules setup

[Basil L. Contovounesios]

Stefan Kangas <stefan@marxist.se> writes:

> How about the attached patch?

I haven't followed the discussion, just noticed some minor issues:

>  (defcustom align-dq-string-modes
>    (append align-lisp-modes align-c++-modes align-perl-modes
> -	  '(python-mode))
> +	  '(python-mode 'vhdl-mode))
                        ^
Stray quote?

> +(make-obsolete-variable 'align-vhdl-rules-list "not used" "27.1")
                                                  ^^^^^^^^^^
This produces the following in *Help*:

  This variable is obsolete since 27.1;
  not used

I think "it is no longer used." or similar, with trailing full stop,
would read better.

>  (defun align-set-vhdl-rules ()
>    "Setup the `align-mode-rules-list' variable for `vhdl-mode'."
> +  (define (obsolete nil "27.1"))
      ^^^^^^
declare?

Thanks,

-- 
Basil

bug#6207: 23.1; align.el vhdl rules setup

[Stefan Kangas]

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

Basil L. Contovounesios <contovob@tcd.ie> writes:
>
> Stefan Kangas <stefan@marxist.se> writes:
>
> > How about the attached patch?
>
> I haven't followed the discussion, just noticed some minor issues:

Ugh.  Thanks for spotting these issues.  I'm a bit sick and tired
today, so maybe I should just stay away from coding for the day...

I've attached an updated patch.

Best regards,
Stefan Kangas

[-- Attachment #2: 0001-Declare-unused-vhdl-code-in-align.el-obsolete.patch --]
[-- Type: text/x-patch, Size: 2015 bytes --]

From f15dcd79a4535bfaa9aee386d280da74ecd2b067 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Thu, 26 Sep 2019 12:31:37 +0200
Subject: [PATCH] Declare unused vhdl code in align.el obsolete

* lisp/align.el (align-vhdl-rules-list, align-set-vhdl-rules): Declare
obsolete.  (Bug#6207)
(align-dq-string-modes, align-open-comment-modes): Move vhdl-mode to
definition instead of adding it later.
---
 lisp/align.el | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/lisp/align.el b/lisp/align.el
index cd72d52df4..c22ff85d2e 100644
--- a/lisp/align.el
+++ b/lisp/align.el
@@ -206,7 +206,7 @@ align-text-modes
 
 (defcustom align-dq-string-modes
   (append align-lisp-modes align-c++-modes align-perl-modes
-	  '(python-mode))
+	  '(python-mode vhdl-mode))
   "A list of modes where double quoted strings should be excluded."
   :type '(repeat symbol)
   :group 'align)
@@ -219,7 +219,7 @@ align-sq-string-modes
 
 (defcustom align-open-comment-modes
   (append align-lisp-modes align-c++-modes align-perl-modes
-	  '(python-mode makefile-mode))
+	  '(python-mode makefile-mode vhdl-mode))
   "A list of modes with a single-line comment syntax.
 These are comments as in Lisp, which have a beginning, but end with
 the line (i.e., `comment-end' is an empty string)."
@@ -842,18 +842,14 @@ align-vhdl-rules-list
   "Alignment rules for `vhdl-mode'.  See `align-rules-list' for more info."
   :type align-rules-list-type
   :group 'align)
-
 (put 'align-vhdl-rules-list 'risky-local-variable t)
+(make-obsolete-variable 'align-vhdl-rules-list "no longer used" "27.1")
 
 (defun align-set-vhdl-rules ()
   "Setup the `align-mode-rules-list' variable for `vhdl-mode'."
+  (declare (obsolete nil "27.1"))
   (setq align-mode-rules-list align-vhdl-rules-list))
 
-(add-hook 'vhdl-mode-hook 'align-set-vhdl-rules)
-
-(add-to-list 'align-dq-string-modes 'vhdl-mode)
-(add-to-list 'align-open-comment-modes 'vhdl-mode)
-
 ;;; User Functions:
 
 ;;;###autoload
-- 
2.20.1

bug#6207: 23.1; align.el vhdl rules setup

[Basil L. Contovounesios]

Stefan Kangas <stefan@marxist.se> writes:

> Basil L. Contovounesios <contovob@tcd.ie> writes:
>>
>> Stefan Kangas <stefan@marxist.se> writes:
>>
>> > How about the attached patch?
>>
>> I haven't followed the discussion, just noticed some minor issues:
>
> Ugh.  Thanks for spotting these issues.  I'm a bit sick and tired
> today, so maybe I should just stay away from coding for the day...

Hope you feel better soon!

> I've attached an updated patch.

Thanks, just one remaining nit:

> +(make-obsolete-variable 'align-vhdl-rules-list "no longer used" "27.1")
                                                                ^^
CURRENT-NAME should end with a full stop; see docstring of make-obsolete.

-- 
Basil

bug#6207: 23.1; align.el vhdl rules setup

[Stefan Kangas]

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

Basil L. Contovounesios <contovob@tcd.ie> writes:

> > Ugh.  Thanks for spotting these issues.  I'm a bit sick and tired
> > today, so maybe I should just stay away from coding for the day...
>
> Hope you feel better soon!

Thanks - I feel much better today. :-)

> > I've attached an updated patch.
>
> Thanks, just one remaining nit:
>
> > +(make-obsolete-variable 'align-vhdl-rules-list "no longer used" "27.1")
>                                                                 ^^
> CURRENT-NAME should end with a full stop; see docstring of make-obsolete.

Fixed in the attached patch.  If there are no further comments, I
intend to commit this in a couple of days.

Best regards,
Stefan Kangas

[-- Attachment #2: 0001-Declare-unused-vhdl-code-in-align.el-obsolete.patch --]
[-- Type: text/x-patch, Size: 2483 bytes --]

From 042b62e6bb208914f14a92a278ce3e2c89e34ea8 Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefankangas@gmail.com>
Date: Thu, 26 Sep 2019 12:31:37 +0200
Subject: [PATCH] Declare unused vhdl code in align.el obsolete

* lisp/align.el (align-vhdl-rules-list, align-set-vhdl-rules): Declare
obsolete.  (Bug#6207)
(align-dq-string-modes, align-open-comment-modes): Move vhdl-mode to
definition instead of adding it later.
---
 lisp/align.el | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/lisp/align.el b/lisp/align.el
index cd72d52df4..d83f8341c9 100644
--- a/lisp/align.el
+++ b/lisp/align.el
@@ -206,7 +206,7 @@ align-text-modes
 
 (defcustom align-dq-string-modes
   (append align-lisp-modes align-c++-modes align-perl-modes
-	  '(python-mode))
+          '(python-mode vhdl-mode))
   "A list of modes where double quoted strings should be excluded."
   :type '(repeat symbol)
   :group 'align)
@@ -219,7 +219,7 @@ align-sq-string-modes
 
 (defcustom align-open-comment-modes
   (append align-lisp-modes align-c++-modes align-perl-modes
-	  '(python-mode makefile-mode))
+          '(python-mode makefile-mode vhdl-mode))
   "A list of modes with a single-line comment syntax.
 These are comments as in Lisp, which have a beginning, but end with
 the line (i.e., `comment-end' is an empty string)."
@@ -805,9 +805,7 @@ align-highlight-overlays
 (defvar align-regexp-history nil
   "Input history for the full user-entered regex in `align-regexp'")
 
-;; Sample extension rule set, for vhdl-mode.  This should properly be
-;; in vhdl-mode.el itself.
-
+;; Sample extension rule set for vhdl-mode.  This is now obsolete.
 (defcustom align-vhdl-rules-list
   `((vhdl-declaration
      (regexp   . "\\(signal\\|variable\\|constant\\)\\(\\s-+\\)\\S-")
@@ -842,18 +840,14 @@ align-vhdl-rules-list
   "Alignment rules for `vhdl-mode'.  See `align-rules-list' for more info."
   :type align-rules-list-type
   :group 'align)
-
 (put 'align-vhdl-rules-list 'risky-local-variable t)
+(make-obsolete-variable 'align-vhdl-rules-list "no longer used." "27.1")
 
 (defun align-set-vhdl-rules ()
   "Setup the `align-mode-rules-list' variable for `vhdl-mode'."
+  (declare (obsolete nil "27.1"))
   (setq align-mode-rules-list align-vhdl-rules-list))
 
-(add-hook 'vhdl-mode-hook 'align-set-vhdl-rules)
-
-(add-to-list 'align-dq-string-modes 'vhdl-mode)
-(add-to-list 'align-open-comment-modes 'vhdl-mode)
-
 ;;; User Functions:
 
 ;;;###autoload
-- 
2.20.1

bug#6207: 23.1; align.el vhdl rules setup

[Stefan Kangas]

close 6207 27.1
quit

Stefan Kangas <stefan@marxist.se> writes:

> If there are no further comments, I
> intend to commit this in a couple of days.

No further comments in 6 days; pushed as commit dd3592564a.  Closing.

Best regards,
Stefan Kangas