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

Fix #124: Treat :character offsets as UTF-16 code units #125

Closed
wants to merge 1 commit into from
Closed
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
49 changes: 39 additions & 10 deletions eglot.el
Original file line number Diff line number Diff line change
Expand Up @@ -721,23 +721,52 @@ CONNECT-ARGS are passed as additional arguments to
(let ((warning-minimum-level :error))
(display-warning 'eglot (apply #'format format args) :warning)))

(defvar eglot-full-position-conversion t
"Whether positions are calculated in full compliance with the standard.
Copy link
Owner

Choose a reason for hiding this comment

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

"If non-nil, calculate positions in full compliance with standard"

Setting this to nil may improve performance, but it can also
introduce bugs when characters wider than two UTF-16 code units
Copy link
Owner

Choose a reason for hiding this comment

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

You probably meant: "wider than one".

are used.")

(defun eglot--count-characters (beg end)
Copy link
Owner

Choose a reason for hiding this comment

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

I'd name this "eglot--lsp-chars-between"

"Get the number of LSP characters in region BEG END.
If `eglot-full-position-conversion' is non-nil, then convert the
region to UTF-16 and count the number of code units. Otherwise
return the distance between BEG and END."
(cond
Copy link
Owner

Choose a reason for hiding this comment

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

You don't need so many newlines, it makes reading very strenuous, for me at least.

(eglot-full-position-conversion
(/
(-
(length (encode-coding-region beg end 'utf-16 t))
;; The first two bytes are utf-16 signature.
2)
;; Each code unit takes two bytes, so divide by two.
2))
(t (- end beg))))

(defun eglot--pos-to-lsp-position (&optional pos)
"Convert point POS to LSP position."
(eglot--widening
(list :line (1- (line-number-at-pos pos t)) ; F!@&#$CKING OFF-BY-ONE
:character (- (goto-char (or pos (point)))
(line-beginning-position)))))
(and pos (goto-char pos))
(list :line (1- (line-number-at-pos (point) t))
:character
(eglot--count-characters (line-beginning-position) (point)))))

(defun eglot--lsp-position-to-point (pos-plist &optional marker)
"Convert LSP position POS-PLIST to Emacs point.
If optional MARKER, return a marker instead"
(save-excursion (goto-char (point-min))
(forward-line (min most-positive-fixnum
(plist-get pos-plist :line)))
(forward-char (min (plist-get pos-plist :character)
(- (line-end-position)
(line-beginning-position))))
(if marker (copy-marker (point-marker)) (point))))
(save-excursion
(goto-char (point-min))
(forward-line (min most-positive-fixnum
(plist-get pos-plist :line)))
(let ((lsp-pos (min
(plist-get pos-plist :character)
(eglot--count-characters
(line-beginning-position) (line-end-position))))
(pos 0))
(while (< pos lsp-pos)
(cl-incf pos (eglot--count-characters (point) (1+ (point))))
Copy link
Owner

Choose a reason for hiding this comment

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

This whole section is extremely hacky, counting character lengths one by one, it's like you're reimplementing Emacs's coding systems.

(forward-char 1)))
(if marker (copy-marker (point-marker)) (point))))

(defun eglot--path-to-uri (path)
"URIfy PATH."
Expand Down