wwwjfy / emacs-fish

fish-mode for emacs
99 stars 15 forks source link

indentation problem #24

Closed c02y closed 7 years ago

c02y commented 9 years ago

https://github.com/d12frosted/fish-mode/issues/2

wwwjfy commented 9 years ago

d12frosted has committed some fixes. It's not easy to match all the cases without understanding the semantics.

The second problem you met, I think, is caused by the "switch" keyword matched.

d12frosted commented 9 years ago

Yeah, I did a miser attempt to fix those problems, but as @wwwjfy says, it's hard to match all cases without understanding the semantics. So probably I'll try to move into that direction.

As for switch keyword - @wwwjfy is right. This is what breaks indentation. And my last fix is not for such cases. Which is bad :(

alphapapa commented 9 years ago

Would it be a good idea to try to use the Emacs shell-script-mode for Fish? I looked at it one time, and it looked rather complicated, but it is designed to handle a variety of shell script syntaxes. It might be easier than getting all the edge cases right from scratch.

wwwjfy commented 9 years ago

@alphapapa I'm afraid not. shell-script-mode (a.k.a sh-mode) is for sh/bash/zsh, with the syntax and indentation, which is different with fish.

c02y commented 9 years ago

"match all the cases" shouldn't be the solution, use semantics should.

zhengpd commented 7 years ago

Any progress on this? I got into similar problem with following code:

 set -gx fish_function_path $my-path $fish_function_path
 set -gx fish_function_path $other-path $fish_function_path

The 2nd line is always indented 1 more level than 1st line in fish-mode, but they should be the same level. If I change fish_function_path to fish_xxx_path then the problem goes away. So the function keyword is the cause here.

alphapapa commented 7 years ago

@zhengpd Might be that these regexps need to be adjusted:

(defvar fish/block-opening-terms
  (mapconcat
   'identity
   '("\\<if\\>"
     "\\<function\\>"
     "\\<while\\>"
     "\\<for\\>"
     "\\<begin\\>"
     "\\<switch\\>")
   "\\|"))

The \> sequence means end-of-word, and I think that will match the function in fish_function_path. However, \_> means end-of-symbol, which should not match there. So I guess all of these beginning/end-of-word sequences should be changed to beginning/end-of-symbol. See https://www.gnu.org/software/emacs/manual/html_node/elisp/Regexp-Backslash.html

Here:

(defvar fish/block-opening-terms
  (rx symbol-start
      (or "if"
          "function"
          "while"
          "for"
          "begin"
          "switch")
      symbol-end))

Please try that and let me know if it fixes the problem. (Use C-M-x in the defvar to evaluate it and redefine the existing var.)

wwwjfy commented 7 years ago

Right, and I've tested and pushed it.

alphapapa commented 7 years ago

@wwwjfy Note that it would be good to use the rx macro, because it makes optimized regexps.

wwwjfy commented 7 years ago

@alphapapa Fixed. Thanks for your suggestion :)

zhengpd commented 7 years ago

@alphapapa @wwwjfy Just upgraded to latest version and found no problem. Thanks both.

alphapapa commented 7 years ago

@wwwjfy I don't mean to pester you, but the way you wrote the rx doesn't result in optimized regexps. :)

Here they are compared:

(rx
 (or (and symbol-start "if" symbol-end)
     (and symbol-start "function" symbol-end)
     (and symbol-start "while" symbol-end)
     (and symbol-start "for" symbol-end)
     (and symbol-start "begin" symbol-end)
     (and symbol-start "switch" symbol-end)
     )) ;; => "\\_<if\\_>\\|\\_<function\\_>\\|\\_<while\\_>\\|\\_<for\\_>\\|\\_<begin\\_>\\|\\_<switch\\_>"

(rx symbol-start
    (or "if"
        "function"
        "while"
        "for"
        "begin"
        "switch")
    symbol-end) ;; => "\\_<\\(?:\\(?:begin\\|f\\(?:or\\|unction\\)\\|if\\|switch\\|while\\)\\)\\_>"
wwwjfy commented 7 years ago

@alphapapa Ah, you meant that. I wondered how the result of rx (my version) was optimized since it had the same result as the hand-writing version, but I forgot to ask. It looks much better this way now, Thanks very much~

c02y commented 7 years ago

I found another indentation problem. This line is in my config.fish file.

set -l msg 'This should not happen. Have you changed the cd function?'

all the lines after this will not be aligned with this line but more 4 spaces at the beginning like:

set -l msg 'This should not happen. Have you changed the cd function?'
    printf (_ "$msg\n")

And I found that the word "function" cannot be put between the '' symbol, try it yourself, just use:

set -l msg 'function'

the indentation of all the following lines will be wrong. That is weird.

wwwjfy commented 7 years ago

@c02y Yes, by the current naive implementation, function is no different to be in a string or as a keyword of fish. I don't have time or ability to fix this in near future. Sorry, but you may have to work around it.

alphapapa commented 7 years ago

I just pushed a branch that I think fixes this problem. Please test and let me know. :)

wwwjfy commented 7 years ago

@alphapapa Thanks, I didn't know we can do that. I found an issue in your code, though. In (goto-char (match-beginning 0)), (match-beginning 0) returns the position from the beginning of the string, not the whole buffer. I think something like (line-beginning-position) should be added.

alphapapa commented 7 years ago

Hm, I think I see the problem, and I'm surprised it worked. Going to step through it in a moment, but I think to fix this maybe we need to refactor into a function that returns the number of tokens on the current line, rather than one that takes a string.

alphapapa commented 7 years ago

Ok, I pushed another commit that simplifies the implementation. Seems to be working fine for me. Please test and let me know. :)

alphapapa commented 7 years ago

Sorry for the commit noise, should have tidied it up before pushing it the first time. Anyway, I think this last commit should be good.

alphapapa commented 7 years ago

Also went ahead and committed test-syntax.fish, which helps to verify that everything's working.

alphapapa commented 7 years ago

I'll just post it as a PR...