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

option to restore previous TAB behavior #196

Merged
merged 9 commits into from
May 6, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions julia-mode-tests.el
Original file line number Diff line number Diff line change
Expand Up @@ -1009,6 +1009,49 @@ hello world
(string-to-syntax "\\")
(syntax-after 13)))))

;;; testing julia-latexsub-or-indent

(cl-defun julia-test-latexsub-or-indent (from &key (position (1+ (length from))) (greedy t))
"Utility function to test `julia-latexsub-or-indent'.

This is how it works:

1. FROM is inserted in a buffer.

2. The point is moved to POSITION.

3. `julia-latexsub-or-indent' is called on the buffer.

If `julia-latexsub-selector' is called, it selects the first replacement, which is also placed in SELECTION (otherwise it is NIL).

Return a cons of the

1. buffer contents

2. the replacement of SELECTION when not nil.

The latter can be used to construct test comparisons."
(let* ((selection)
(julia-latexsub-selector
(lambda (replacements)
(setf selection (car replacements))
selection))
(julia-latexsub-greedy greedy))
(cons (with-temp-buffer
(insert from)
(goto-char position)
(julia-latexsub-or-indent t)
(buffer-string))
(gethash selection julia-mode-latexsubs))))

(ert-deftest julia--test-latexsub-or-indent ()
(should (equal (julia-test-latexsub-or-indent "\\circ") '("∘")))
(let ((result (julia-test-latexsub-or-indent "\\circXX" :position 5)))
(should (equal (car result) (concat (cdr result) "cXX"))))
(let ((result (julia-test-latexsub-or-indent "\\circ" :greedy nil)))
(should (equal (car result) (cdr result))))
(should (equal (julia-test-latexsub-or-indent "\\alpha") '("α"))))

;;;
;;; run all tests
;;;
Expand Down
94 changes: 76 additions & 18 deletions julia-mode.el
Original file line number Diff line number Diff line change
Expand Up @@ -67,35 +67,57 @@ User can still use `abbrev-mode' or `expand-abbrev' to substitute
unicode for LaTeX even if disabled."
:type 'boolean)

(defcustom julia-latexsub-greedy t
"When `t', `julia-latexsub-or-indent' does not offer options when a complete match is found. Eg for \"\\bar\", \"\\barcap\" etc will not be offered in a prompt."
:type 'boolean)

(defun julia-latexsub-selector-completing-read (replacements)
"Use `completing-read' to pick an item from REPLACEMENTS."
(completing-read "LaTeX completions: " replacements (lambda (&rest _) t) t))

(defvar julia-latexsub-selector 'julia-latexsub-selector-completing-read
"A function that is called when the `julia-latexsub-or-indent' finds multiple matches for a prefix.

The argument is a list of strings. The function should ALWAYS return an item from this list, otherwise an error occurs.

The default implementation uses `completing-read'.")

(defconst julia-mode--latexsubs-partials
(let ((table (make-hash-table :test 'equal)))
(maphash (lambda (latex _subst)
(cl-assert (string= (substring latex 0 1) "\\") nil
"LaTeX substitution does not start with \\.")
(let ((len (length latex)))
(cl-assert (< 1 len) nil "Trivially short LaTeX subtitution")
;; for \foo, put f, fo, foo into the table
(cl-loop for i from 2 to len
do (puthash (substring latex 1 i) t table))))
julia-mode-latexsubs)
table)
(let ((table-unordered (make-hash-table :test 'equal))
(table-ordered (make-hash-table :test 'equal)))
(cl-flet ((_append (key replacement)
(puthash key (cons replacement (gethash key table-unordered nil)) table-unordered)))
;; accumulate partials
(maphash (lambda (latex _unicode)
(cl-assert (string= (substring latex 0 1) "\\") nil
"LaTeX substitution does not start with \\.")
(let ((len (length latex)))
(cl-assert (< 1 len) nil "Trivially short LaTeX subtitution")
;; for \foo, put \f, \fo, \foo into the table
(cl-loop for i from 2 to len
do (_append (substring latex 0 i) latex))))
julia-mode-latexsubs)
;; order by LaTeX part
(maphash (lambda (partial replacements)
(puthash partial (sort replacements #'string<) table-ordered))
table-unordered))
table-ordered)
"A hash table containing all partial strings from the LaTeX abbreviations in
`julia-mode-latexsubs' as keys. Values are always `t', the purpose is to
represent a set.")
`julia-mode-latexsubs' as keys. Values are sorted lists of complete \"\\some_string\".")

(defun julia-mode--latexsubs-longest-partial-end (beg)
"Starting at `beg' (should be the \"\\\"), return the end of the longest
partial match for LaTeX completion, or `nil' when not applicable."
(save-excursion
(goto-char beg)
(when (and (= (char-after) ?\\) (not (eobp)))
(forward-char)
(let ((beg (point)))
(forward-char) ; move past the \
(cl-flet ((next-char-matches? ()
(let* ((end (1+ (point)))
(str (buffer-substring-no-properties beg end))
(valid? (gethash str julia-mode--latexsubs-partials)))
valid?)))
(let* ((end (1+ (point)))
(str (buffer-substring-no-properties beg end))
(valid? (gethash str julia-mode--latexsubs-partials)))
valid?)))
(while (and (not (eobp)) (next-char-matches?))
(forward-char)))
(point)))))
Expand Down Expand Up @@ -919,6 +941,42 @@ buffer where the LaTeX symbol starts."
(abbrev-insert symb name beg end)))
#'ignore))

(defun julia-mode--latexsub-before-point ()
"When there is a LaTeX substitution that can be made before the point, return (CONS BEG LATEX).

`beg' is the position of the `\`, `latex' is the string to replace, including the `\`.

When multiple options match, ask the user to clarify via `julia-latexsub-selector', unless there is a complete match and `julia-latexsub-greedy' is `t'."
(when-let (beg (julia--latexsub-start-symbol))
(let ((partial (buffer-substring-no-properties beg (point))))
(when-let (replacements (gethash partial julia-mode--latexsubs-partials))
(let* ((complete-match (member partial replacements))
(latex (cond
;; complete match w/ greedy
((and complete-match julia-latexsub-greedy) partial)
;; multiple replacements, ask user
((cdr replacements) (funcall julia-latexsub-selector replacements))
;; single replacement, pick that
(t (car replacements)))))
(cons beg latex))))))

(defun julia-latexsub-or-indent (arg)
"Either indent according to Julia mode conventions or perform a LaTeX-like symbol substution.

When multiple options match, ask the user to clarify via `julia-latexsub-selector', unless there is a complete match and `julia-latexsub-greedy' is `t'.

Presently, this is not the default. Enable with eg

(define-key julia-mode-map (kbd \"TAB\") 'julia-latexsub-or-indent)

eg in your `julia-mode-hook'."
(interactive "*i")
(if-let (replacement (julia-mode--latexsub-before-point))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for not reviewing before you merged. I've been meaning to dig into this for months. Could you please comment on why this entire if-let couldn't be replaced with something like:

(let ((completion-at-point-functions '(julia-mode-latexsubs-completion-at-point-before)))
  (unless (completion-at-point)
    (julia-indent-line)))

I'm not immediately seeing why this wouldn't give both better features and integration and significantly more implementation simplicity.

I had meant to do a mock-up PR of this implementation to let others play with it and compare with the one in this PR, but haven't had the mental energy to do so, and I'm not sure when I will, so I'm just posting the idea in case there's an obvious problem I'm missing.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this option is missing this from the OP

option for greedy replacement for a full match (eg \bar will replace, no popup)

but I think you could handle just that case with significantly less complexity than this PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I do not understand. julia-latexsub-greedy, which is set to t by default, does exactly this (eg \bar just completes).

Regarding complexity: I agree that in an ideal world, this PR would not have been necessary. But practically, the generic completion mechanisms of Emacs have proven to be baroque and full of corner cases and/or implementation bugs like #204 (thanks for chasing many of those down!). I think that for now there is value in maintaining our own version which Just Works ™️.

(progn
(delete-backward-char (- (point) (car replacement)))
(insert (gethash (cdr replacement) julia-mode-latexsubs)))
(julia-indent-line)))
Copy link
Contributor

@non-Jedi non-Jedi May 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, if this ultimately calls julia-indent-line, I don't think it solves the problem from #193 at all. The issue in #193 was being able to have TAB work for both latex substitution as well as inserting TABs (see example in this comment). I think this might do what ronisbr was looking for if it called indent-for-tab-command instead of julia-indent-line.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I missed this comment, just found it among messages. This PR solves the problem for me, if anyone still has problems with it please open another issue.

Specifically, it only calls julia-indent-line when it could not make a substitution. So, TAB tries to subtitute, and indents if it can't. I find that behavior useful.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So basically it gives you the reverse of the behavior of indent-for-tab-command when tab-always-indent is set to 'complete:

  • With indent-for-tab-command with (setq tab-always-indent 'complete): indent then complete
  • With this function: complete (but only latexsubs) then indent

If we change julia-indent-line in this function to indent-for-tab-command, the user can change between TAB inserting tabs and TAB indenting the line by customizing the value of tab-always-indent. I'll create a PR for your review when I have a few minutes.

If you haven't seen it yet, could you please comment on my other review comment when you get a chance?


;; Math insertion in julia. Use it with
;; (add-hook 'julia-mode-hook 'julia-math-mode)

Expand Down
Loading