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

Gracefully indenting structs and assembly blocks. #72

Open
hrkrshnn opened this issue Jun 21, 2021 · 14 comments
Open

Gracefully indenting structs and assembly blocks. #72

hrkrshnn opened this issue Jun 21, 2021 · 14 comments

Comments

@hrkrshnn
Copy link
Member

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
Copy link
Member Author

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

@LefterisJP
Copy link
Collaborator

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 😅

But I see you already have an idea!

@coventry
Copy link
Contributor

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
Copy link
Member Author

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

@coventry
Copy link
Contributor

coventry commented Sep 30, 2021

@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
Copy link
Member Author

hrkrshnn commented Oct 4, 2021

@coventry Sure, will do. Thanks.

@clarkhenry
Copy link
Contributor

clarkhenry commented Oct 8, 2021

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
Copy link
Contributor

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

@hrkrshnn
Copy link
Member Author

hrkrshnn commented Oct 26, 2021

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

@TurtleHive
Copy link

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)))
  • looking-at-p so we don't mess with match data
  • interactive is not necessary
  • quoting lambdas is negative

@LefterisJP
Copy link
Collaborator

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
Copy link
Contributor

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
Copy link
Contributor

@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
Copy link
Contributor

taquangtrung commented Dec 2, 2022

@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))
  }
}

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

No branches or pull requests

6 participants