Closed gmlarumbe closed 2 years ago
Can you please separately make a pull with the "tests" changes to add the semicolon, and resulting test_ok fixes, and once passes push that change.
We'll need some tests that uses verilog-align-declaration-comments nil (set with appropriate Local Variables at bottom).
I squashed the PR changes into the semicolon tests commit and added two more tests:
Commit ef566ce does not untabify comments in order to align them and untabifies added/modified lines of code. I think it could now be ready for squash merging.
Diff with previous commit of this PR:
diff --git a/tests_ok/automodport_full.v b/tests_ok/automodport_full.v
index 785f08a..6417ba4 100755
--- a/tests_ok/automodport_full.v
+++ b/tests_ok/automodport_full.v
@@ -11,8 +11,8 @@ module auto_module
output out_pure,
input in_pure,
// End of automatics
- //ex: input in_pure;
- //ex: output out_pure;
+ //ex: input in_pure;
+ //ex: output out_pure;
/*AUTOINOUTMODPORT("automodport_if" "req_mon_mp")*/
// Beginning of automatic in/out/inouts (from modport)
@@ -20,9 +20,9 @@ module auto_module
input [63:0] req_dat,
input req_credit,
// End of automatics
- //ex: input req_credit, // To auto_i of auto_intf.sv
- //ex: input [63:0] req_data, // To auto_i of auto_intf.sv
- //ex: input req_val, // To auto_i of auto_intf.sv
+ //ex: input req_credit, // To auto_i of auto_intf.sv
+ //ex: input [63:0] req_data, // To auto_i of auto_intf.sv
+ //ex: input req_val, // To auto_i of auto_intf.sv
/*AUTOINOUTMODPORT("automodport_if" "rsp_drv_mp")*/
// Beginning of automatic in/out/inouts (from modport)
diff --git a/tests_ok/autotieoff_signed.v b/tests_ok/autotieoff_signed.v
index 49d1f51..d76f619 100644
--- a/tests_ok/autotieoff_signed.v
+++ b/tests_ok/autotieoff_signed.v
@@ -63,7 +63,7 @@ module autotieoff_signed (/*AUTOARG*/
// Beginning of automatic unused inputs
ExtraIn,
// End of automatics
- 1'b0};
+ 1'b0 } ;
// lint_checking SCX_UNUSED OFF
endmodule
diff --git a/verilog-mode.el b/verilog-mode.el
index a150b71..96fac06 100644
--- a/verilog-mode.el
+++ b/verilog-mode.el
@@ -7289,8 +7289,8 @@ Be verbose about progress unless optional QUIET set."
(setq comm-ind (verilog-get-comment-align-indent (marker-position startpos) endpos))
(save-excursion
(goto-char (marker-position startpos))
- (while (progn (setq e (marker-position endpos))
- (< (point) e))
+ (while (progn (setq e (marker-position endpos))
+ (< (point) e))
(when (verilog-search-comment-in-declaration e)
(goto-char (match-beginning 0))
(delete-horizontal-space)
@@ -7524,8 +7524,8 @@ BEG and END."
"Move cursor to position of comment in declaration and return point.
BOUND is a buffer position that bounds the search."
(and (verilog-re-search-forward
- (or (and verilog-indent-declaration-macros
- verilog-declaration-re-1-macro)
+ (or (and verilog-indent-declaration-macros
+ verilog-declaration-re-1-macro)
verilog-declaration-or-iface-mp-re-2-no-macro) bound 'move)
(not (looking-at (concat "\\s-*" verilog-comment-start-regexp)))
(re-search-forward verilog-comment-start-regexp (point-at-eol) :noerror)))
@@ -7533,18 +7533,17 @@ BOUND is a buffer position that bounds the search."
(defun verilog-get-comment-align-indent (b endpos)
"Return the indent level that will line up comments within the region.
Region is defined by B and ENDPOS."
- (untabify b endpos) ; Needed for proper point calculations
(save-excursion
(let ((ind 0)
e comm-ind)
(goto-char b)
;; Get rightmost position
(while (progn (setq e (marker-position endpos))
- (< (point) e))
+ (< (point) e))
(when (verilog-search-comment-in-declaration e)
(end-of-line)
(verilog-backward-syntactic-ws)
- (setq comm-ind (1+ (- (point) (point-at-bol))))
+ (setq comm-ind (1+ (current-column)))
(when (> comm-ind ind)
(setq ind comm-ind)))
(forward-line 1))
Hi,
This PR adds support to automatically align comments in declarations after running
verilog-pretty-declarations
.Related issues: #435 #898 #922 #1157
The feature is enabled by default and controlled by variable
verilog-align-declaration-comments
in case user wants to disable it to keep previous behavior. I have run all the tests in 91b5f59 with this variable set to nil and everything seemed to work fine.The first commit includes all the changes in Elisp source and comment aligning relevant Verilog tests. The second one groups changes of 5 tests that I believe would need double checking. This feature produces changes in many tests and although I have reviewed all of them I might also have missed something important. Once they are reviewed by someone else I can squash both commits and merge it into master.
Cheers!