ethereum / emacs-solidity

The official solidity-mode for EMACS
GNU General Public License v3.0
206 stars 66 forks source link

Gracefully indenting structs and assembly blocks. #72

Open hrkrshnn opened 3 years ago

hrkrshnn commented 3 years ago

Currently, the package expects a semicolon after a struct declaration, and also expects semicolons in assembly blocks.

For example

struct s {
    uint x;
}
// emacs expects semicolon here
// so automatically does an extra indent here
function f() {
    assembly {
        sstore(0, 0)
       // emacs expects a semicolon in the line before, so automatically makes an extra indent here again.
    }
}

I assume the struct issue is somehow related to c-mode or cc-mode that solidity-mode derives from.

I had a look at https://www.gnu.org/software/emacs/manual/html_mono/ccmode.html. Unfortunately, I can't find an obvious fix.

Any idea how this can be fixed?

hrkrshnn commented 3 years ago

Looks like cc-cmds.el should help here. I'll take a look this week.

LefterisJP commented 3 years ago

I assume the struct issue is somehow related to c-mode or cc-mode that solidity-mode derives from.

Hey @hrkrshnn yes this is definitely due to the inheritance from c-mode.

Any idea how this can be fixed?

Not really. Havent' worked on inmproving this package in a long time :sweat_smile:

But I see you already have an idea!

coventry commented 2 years ago

This is a bit hacky, but seems to work with cursory testing. It at least outlines one possible approach. Happy to make a PR to emacs-solidity if there's sufficient interest.

;; solidity ends struct blocks without a semicolon, but c-mode indentation
;; expects struct blocks to have semicolons. This function returns true if we
;; are at the end of a struct block. It is to be used with c-at-vsemi-p-fn, to
;; tell c-mode that there is a "virtual semicolon" after the struct block, so
;; that it indents a subsequent statement correctly, as a 'statement rather than
;; a 'statement-cont.
(defun solidity-at-vsemi-p (&optional pos)
  (let ((rpos (or pos (point))))
    (save-excursion
      (goto-char rpos)
      (ignore-errors
        ;; Try to jump back to the word "struct", as if we're at the end of a
        ;; syntactically-correct struct
        (backward-sexp) ;; struct body
        (backward-sexp) ;; struct name
        (backward-sexp) ;; the keyword "struct"
        (looking-at "\\bstruct\\b")
        )
      )
    )
  )

Currently enabled by (setq c-at-vsemi-p-fn 'solidity-at-vsemi-p) in a solidity-mode-hook.

hrkrshnn commented 2 years ago

@coventry please make a PR. I would love to have the fix.

coventry commented 2 years ago

@hrkrshnn , would you mind adding the solidity-at-vsemi-p function from above and

(add-hook 'solidity-mode-hook '(lambda () (interactive) (setq c-at-vsemi-p-fn 'solidity-at-vsemi-p)))

to your ~/.emacs (or whatever configuration file you usually use) and trying it out for a week or so? I'll be doing the same, but it would be good to have someone else using it, too. I think the solution I've given is potentially a bit fragile, so we should see whether we can break it with casual use.

hrkrshnn commented 2 years ago

@coventry Sure, will do. Thanks.

clarkhenry commented 2 years ago

Would also love this fix! I'm finding myself hacking cc-cmds more and more to try to comply with the Solidity Style Guide.

Will be trying this solidity-at-vsemi-p function in my config as a work-around as well.

I use a fair about of inline assembly as well. Should solidity-at-vsemi-p work just the same updating the regex to something like (looking-at "\\(\\bstruct\\b\\|\\bassembly\\b\\)")? This is not working for me (except on structs). An assembly block is somewhat different, where inline operations do not terminate with a ;.

Thanks!

coventry commented 2 years ago

@hrkrshnn , have you had any issues running this change?

hrkrshnn commented 2 years ago

@coventry I didn't have any problems so far. Would you like to submit a PR?

TurtleHive commented 2 years ago

I've been bothered by this issue as well, and the hack above fixes it for me too. Thanks a lot for this @coventry!

Some minor suggested adjustments if someone ends up creating a PR for it.


(defun solidity-at-vsemi-p (&optional pos)
  (let ((rpos (or pos (point))))
    (save-excursion
      (goto-char rpos)
      (ignore-errors
        ;; Try to jump back to the word "struct", as if we're at the end of a
        ;; syntactically-correct struct. Struct body, struct name, the keyword "struct".
        (forward-sexp -3)
        (looking-at-p "\\bstruct\\b")))))

(add-hook 'solidity-mode-hook
          (lambda () (setq-local c-at-vsemi-p-fn 'solidity-at-vsemi-p)))
LefterisJP commented 2 years ago

Would either @coventry or you @TurtleHive like to make a PR for this? Would be cool to have it inside the upstream of the package.

clarkhenry commented 2 years ago

None of this works for assembly blocks, right?

I think it makes sense to either: rename this issue / PR (and open a separate issue for supporting assembly blocks), or modifying the elisp to work for assembly blocks as well.

coventry commented 2 years ago

@LefterisJP, I'm not comfortable putting this in production code, because it's a hack. If someone else wants to take responsibility for it, they should go for it.

taquangtrung commented 1 year ago

@clarkhenry: I encountered the same issue and here is my hack to align statements inside an assembly block:

;; Hacks to fix indentation inside `assembly' block
(defun solidity-assembly-at-vsemi-p (&optional pos)
    (let ((rpos (or pos (point))))
      (save-excursion
        (goto-char rpos)
        (let ((in-assembly nil)
              (paren-start-pos (cadr (syntax-ppss))))
          (while (and paren-start-pos (not in-assembly))
            (if (looking-back "assembly\\(\s\\|\n\\)*")
                (setq in-assembly t)
              (setq paren-start-pos (cadr (syntax-ppss (1- paren-start-pos))))))
          in-assembly))))

  (add-hook 'solidity-mode-hook
            (lambda () (setq-local c-at-vsemi-p-fn 'solidity-assembly-at-vsemi-p)))

It can indent correctly this code:

assembly {
  let a := "test"
  let b := hex"112233445566778899aabbccddeeff6677889900"
  let c := hex"1234_abcd"
  sstore(0, 0)
  sstore(0, 0)
  sstore(0, 0)
}

But the indentation on for loop is still awkward:

assembly {
  // Iterate until the bound is not met.
  for {
    let end := add(dataElementLocation, mul(len, 0x20))
  } lt(dataElementLocation, end) {
  dataElementLocation := add(dataElementLocation, 0x20)
  } {
  sum := add(sum, mload(dataElementLocation))
  }
}