idris-hackers / idris-mode

Idris syntax highlighting, compiler-supported editing, interactive REPL and more things for Emacs.
GNU General Public License v3.0
269 stars 71 forks source link

idris-add-clause and friends don't do the right thing with subword-mode enabled #433

Closed tsdh closed 7 years ago

tsdh commented 8 years ago

With Emacs' subword-mode enabled (which makes word movement commands stop at CamelHumps), all functions using idris-thing-at-point don't work correctly on CamelCase words. For example, typing C-c C-s on a definition quickSort will either create a body for quick or Sort depending on where point is on.

I'd rather just use (current-word t t) for implementing idris-thing-at-point. I don't see why where that would make a difference and why you try (thing-at-point 'word) and then (thing-at-point 'symbol). I've tried all three on different Idris symbols (CamelCaseThings, camelCaseThings, stuff_with_underscore, ?holes_like_this), and all three always return the same thing except for (thing-at-point 'word) in the scenario where subword-mode is enabled.

current-word works according to the current buffer's syntax table whereas thing-at-point essentially uses movement commands (e.g., forward-word, backward-word) and so doesn't work for the purpose of identifying symbols when movement is customized by things like subword-mode or superword-mode.

Long story short: please use (current-word t t) rather than thing-at-point unless there's some important reason I am missing. If you want, I can also provide a PR.

david-christiansen commented 8 years ago

Thanks for pointing this out! If we can adopt something that will work for you, while simultaneously fixing a bug that I found while investigating this, it would be good.

(current-word t t) fails on operators, such as ++++, but (current-word t) returns them just fine. However, both the current idris-thing-at-point and (current-word t) return ++++fnord when run with point over a plus on oops ++++fnord. Can you think of an operator that would correctly return ++++?

david-christiansen commented 8 years ago

(I suppose a hand-hacked regexp mess is one possibility, but Emacs has lots of good tools, so it'd be better to avoid that)

tsdh commented 8 years ago

@david-christiansen I just checked what c-mode does to know that x+y is an operator and two symbols and not just one symbol. It gives all operators (.&|+-*/<>...) punctuation syntax.

I'd lean towards doing the same for Idris. Then this function would return the current symbol or operator (and line number).

(defun idris-thing-at-point ()
  "Return the line number and name at point. Use this in Idris source buffers."
  (let ((line (idris-get-line-num)))
    (cons
     (if (equal (syntax-after (point))
                (string-to-syntax "."))
         ;; We're on an operator.
         (save-excursion
           (skip-syntax-backward ".")
           (let ((beg (point)))
             (skip-syntax-forward ".")
             (buffer-substring-no-properties beg (point))))
       ;; Try if we're on a symbol or fail otherwise.
       (or (current-word t)
           (error "Nothing identifiable under point")))
     line)))
tsdh commented 8 years ago

If you agree that's the right thing to do, I can submit a PR.

david-christiansen commented 8 years ago

I'm very happy if you submit a PR that makes idris-mode compatible with more Emacs features, and improves the lookup commands as well! I'm quite short on time at the moment, so please ping me if I lose track of a PR or something. Sorry in advance!

david-christiansen commented 8 years ago

And thanks for engaging!

tsdh commented 8 years ago

You're welcome. I'll see when I come to this, but surely at the weekend.

tsdh commented 8 years ago

@david-christiansen Ok, wife and kid are still not at home so I had some spare minutes to fix the issue. Thereby I also found that -- single line comments were not highlighted due to a syntax issue and fixed that, too.

So if you merge that PR, this issue can be closed.