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

Smart indenting multi-line `define #1082

Open
veripoolbot opened this issue Aug 16, 2016 · 5 comments
Open

Smart indenting multi-line `define #1082

veripoolbot opened this issue Aug 16, 2016 · 5 comments
Labels

Comments

@veripoolbot
Copy link
Collaborator

@veripoolbot veripoolbot commented Aug 16, 2016


Author Name: Kaushal Modi
Original Redmine Issue: 1082 from https://www.veripool.org
Original Date: 2016-08-16


For auto-indentation does not handle `define macros that extend multiple-lines.

Example (manually indented code before calling @indent-region@):

`define drive_agt(AGT_ID) \
  begin \
      some_agt_seq seq; \
      seq = some_agt_seq::type_id::create \
                 (.name({"some_agt_seq_",$sformatf("%0d", AGT_ID)}), \
                  .contxt(get_full_name())); \
      seq.start(env.adc_agt[AGT_ID].sqr_l1); \
  end

Here is what calling @indent-region@ over that code does!:

`define drive_agt(AGT_ID) \
begin \
    some_agt_seq seq; \
      seq = some_agt_seq::type_id::create \
            (.name({"some_agt_seq_",$sformatf("%0d", AGT_ID)}), \
             .contxt(get_full_name())); \
            seq.start(env.adc_agt[AGT_ID].sqr_l1); \
            end

It would be nice if multi-line defines were handled well.

My work-around is to remove the trailing @@ characters, indent the code and then add them back. But I need to remember to not indent the whole buffer for files containing such `defines.

@veripoolbot
Copy link
Collaborator Author

@veripoolbot veripoolbot commented Aug 23, 2016


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2016-08-23T11:06:15Z


Thanks, this is a good suggestion.

Just so you know, the indentation code is pretty fragile and this would add more complication and perhaps performance issues, so it's unlikely to be fixed soon unless you take it on yourself.

@veripoolbot
Copy link
Collaborator Author

@veripoolbot veripoolbot commented Aug 23, 2016


Original Redmine Comment
Author Name: Kaushal Modi
Original Date: 2016-08-23T16:15:52Z


Is not trying to indent the lines ending in @\$@ at all a viable 'solution'?

@veripoolbot
Copy link
Collaborator Author

@veripoolbot veripoolbot commented Aug 25, 2016


Original Redmine Comment
Author Name: Wilson Snyder (@wsnyder)
Original Date: 2016-08-25T10:51:49Z


Is not trying to indent the lines ending in \$ at all a viable 'solution'?

A reasonable thing to be tried.

@veripoolbot
Copy link
Collaborator Author

@veripoolbot veripoolbot commented Aug 25, 2016


Original Redmine Comment
Author Name: Alex Reed
Original Date: 2016-08-25T16:39:50Z


Wilson Snyder wrote:

Is not trying to indent the lines ending in \$ at all a viable 'solution'?

A reasonable thing to be tried.

I tend to agree. I like the idea of semantically-aware indentation inside macros, but it's really not feasible. There's no guarantee that the `define is a complete (System)Verilog statement, therefore we cannot accurately indent the "Right Way" each and every time. Maybe abandoning all attempt at indentation is the best approach.

As a trivial example, consider a multi-line macro that contains quoted text (maybe it's part of a debug/error message). The user most definitely doesn't want their pretty message formatting perturbed by verilog-mode.

@veripoolbot
Copy link
Collaborator Author

@veripoolbot veripoolbot commented Aug 25, 2016


Original Redmine Comment
Author Name: Kaushal Modi
Original Date: 2016-08-25T17:41:31Z


Below tests fine in my personal config .. If that looks fine, I can start working on a patch for @verilog-indent-line-relative@ and @verilog-indent-line@.

(defvar modi/verilog-multi-line-define-line-cache nil
  "Variable set to non-nil if the current line is detected as any but the
last line of a multi-line `define such as:

  `define foo(ARG) \          <- non-nil
     begin \                   <- non-nil
       $display(\"Bar\"); \    <- non-nil
       $display(\"Baz\"); \    <- non-nil
     end                       <- nil
 ")

(defun modi/verilog-selective-indent (&rest args)
  "Return non-nil if point is on certain types of lines.

Non-nil return will happen when the current line is part of a multi-line `define like:
     `define foo(ARG) \
       begin \
         $display(\"Bar\"); \
         $display(\"Baz\"); \
       end

This function is used to tweak the `verilog-mode' indentation functions. "
  (save-excursion
     (beginning-of-line)
     (let* ((is-in-multi-line-define (looking-at "^.*\\\\$")) ; \ at EOL
            (do-not-run-orig-fn (or is-in-multi-line-define
                                    modi/verilog-multi-line-define-line-cache)))
       ;; Cache the current value of `is-in-multi-line-define'
       (setq modi/verilog-multi-line-define-line-cache is-in-multi-line-define)
       do-not-run-orig-fn)))
;; Advise the indentation behavior of `indent-region' done using `C-M-\'
(advice-add 'verilog-indent-line-relative :before-until #'modi/verilog-selective-indent)
;; Advise the indentation done by hitting `TAB'
(advice-add 'verilog-indent-line :before-until #'modi/verilog-selective-indent)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.