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.

Sign up for GitHub

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

option to restore previous TAB behavior #196

Changes from 5 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9d07406
WIP: option to restore previous TAB behavior
tpapp Jun 20, 2023
1c2c9d4
don't need quote in docstring?
tpapp Jun 20, 2023
fef78c5
Add greedy match option.
tpapp Jun 20, 2023
b390203
oops, now we include \\, this should fix tests
tpapp Jun 20, 2023
7b00566
fix
tpapp Jun 20, 2023
4a1a68c
Merge branch 'master' into tp/repl-style-completion
tpapp Apr 10, 2024
86da879
fixes suggested by @dhanak (thanks!)
tpapp Apr 10, 2024
5f2e774
Make selection customizable.
tpapp Apr 10, 2024
9bcca66
add tests, minor corrections
tpapp Apr 10, 2024
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
81 changes: 63 additions & 18 deletions julia-mode.el
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,35 +67,46 @@ 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)

(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 @@ -908,6 +919,40 @@ 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 `completing-read', 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))))
(message "partial %s" partial)
(when-let (replacements (gethash partial julia-mode--latexsubs-partials))
(message "replacements %s" (prin1 replacements))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is cool for debugging purposes, but it bloats the minibuffer unnecessarily, even if temporarily.

This and the previous message line should probably be deleted before merging.

(let* ((complete-match (member partial replacements))
(replacement (cond ((and complete-match julia-latexsub-greedy) partial)
((cdr replacements) (gethash (completing-read "LaTeX completions: " replacements) julia-mode-latexsubs))
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is an error.

When the completion is not full, e.g., \cir, I am offered a list of valid completions. When I press enter on one of them, I get a progn: Wrong type argument: char-or-string-p, nil error.

After some inspection, it seems that this is because this line here should return the raw latex control sequence, not the already substituted value. The substitution is performed in the julia-latexsub-or-indent function.

If I change this line to

((cdr replacements) (completing-read "LaTeX completions: " replacements))

then it works as expected.

(t (car replacements)))))
(cons beg replacement))))))

(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 `completing-read', 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)

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 •
edited
Loading

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