Wrong :beg and :end for source files with CRLF line endings #211

Closed
tstgruby opened this Issue Dec 6, 2011 · 7 comments

Comments

Projects
None yet
6 participants
@tstgruby

tstgruby commented Dec 6, 2011

IProblem apparently reports the begin and and ending of a problem in regard to the byte (character) offset in binary mode.

But Emacs basically strips the CR characters from the file when displaying it in a buffer, so the start and end pointers are line-1 characters off and the wrong region gets highlighted.

Looking into it, I see that you've already tried to fix the position in ensime-make-overlay-at by recalculating beg and end using (the erroneous) positions... Alas, this doesn't work.

Why not pass the line information from ensime-make-note-overlays to ensime-make-overlay-at and only make an overlay spanning the whole line if beg and end are not valid positions (non-integers)?

(defun ensime-make-overlay-at (file line b e msg face)
  "Create an overlay highlighting the given line in
any buffer visiting the given file."
  (let ((beg b)
    (end e))
    (assert (or (integerp line)
        (and (integerp beg) (integerp end))))
    (when-let (buf (find-buffer-visiting file))
      (with-current-buffer buf
    (if (and (integerp beg) (integerp end))
        ;; If DOS eol's, fix the positioning
        ;; Note: this is impossible without the line argument.
        (when (and (integerp line)
               (eq 1 (coding-system-eol-type
                  (buffer-local-value
                   'buffer-file-coding-system buf))))
          (setq beg (- beg (1- line)))
          (setq end (- end (1- line))))

      ;; If line provided, use line to define region
      (save-excursion
        (goto-line line)
        (setq beg (point-at-bol))
        (setq end (point-at-eol)))))

      (ensime-make-overlay beg end msg face nil buf))
    ))



(defun ensime-make-note-overlays (notes)
  (dolist (note notes)
    (destructuring-bind
    (&key severity msg beg end line col file &allow-other-keys) note

      ;; No empty note overlays!
      (when (eq beg end)
    (setq beg (- beg 1)))

      (let ((lang
         (cond
          ((ensime-java-file-p file) 'java)
          ((ensime-scala-file-p file) 'scala)
          (t 'scala)))
        (face
         (cond
          ((equal severity 'error)
           'ensime-errline-highlight)
          (t
           'ensime-warnline-highlight))))

    (when-let (ov (ensime-make-overlay-at
               file line
               (+ beg ensime-ch-fix)
               (+ end ensime-ch-fix)
               msg face))
      (overlay-put ov 'lang lang)
      (push ov ensime-note-overlays))

    ))))

edit: The problem also occurs the other way round: when using e.g. ensime-refactor-rename the transmitted character positions are in emacs' point system and probably would need to be translated...

@martinmcnulty

This comment has been minimized.

Show comment Hide comment
@martinmcnulty

martinmcnulty Aug 22, 2012

This issue also appears to affect syntax-aware selection expanding/contracting, using C-c C-v .

This issue also appears to affect syntax-aware selection expanding/contracting, using C-c C-v .

@aemoncannon

This comment has been minimized.

Show comment Hide comment
@aemoncannon

aemoncannon Aug 22, 2012

Owner

Looks reasonable. Would you be able to make a pull request? I'd like to give you credit in the history.

Thanks

Owner

aemoncannon commented Aug 22, 2012

Looks reasonable. Would you be able to make a pull request? I'd like to give you credit in the history.

Thanks

delexi pushed a commit to delexi/ensime that referenced this issue Dec 10, 2012

Alexander Baier
Correct error-highlighting in CRLF files.
This change is originally authored by tstgruby, i only committed it so
it can be integrated into the ensime source code.
This does not fix problems with expand selection, refactor-rename, etc.
For further information see issue #211.

@delexi delexi referenced this issue Dec 10, 2012

Merged

Issue fix 211 #288

@delexi

This comment has been minimized.

Show comment Hide comment
@delexi

delexi Dec 10, 2012

As this fix has been lying around for at least a year now, i took the changes and put them into a commit. @tstgruby i mentioned you in the commit message, as i could not figure out how to set the author field to a name that did not yet show up in the history. I hope this is ok with you.

I will open a pull request for this issue fix. Please also see #287 for related problems.

PS: I am sorry for the two mentions from two different commits with the same content, i did not realise the first one would still exist. If anyone has a tip on how to remove this, i would be very happy to hear it.

delexi commented Dec 10, 2012

As this fix has been lying around for at least a year now, i took the changes and put them into a commit. @tstgruby i mentioned you in the commit message, as i could not figure out how to set the author field to a name that did not yet show up in the history. I hope this is ok with you.

I will open a pull request for this issue fix. Please also see #287 for related problems.

PS: I am sorry for the two mentions from two different commits with the same content, i did not realise the first one would still exist. If anyone has a tip on how to remove this, i would be very happy to hear it.

@tstgruby

This comment has been minimized.

Show comment Hide comment
@tstgruby

tstgruby Dec 21, 2012

@aemoncannon Sorry, I haven't seen your request to make up a pull request - or it just slipped my mind.

@delexi I'm totally fine with your approach. Thanks for doing this for me!

Please don't care too much about the credit, I'm not utterly bent on it.

@aemoncannon Sorry, I haven't seen your request to make up a pull request - or it just slipped my mind.

@delexi I'm totally fine with your approach. Thanks for doing this for me!

Please don't care too much about the credit, I'm not utterly bent on it.

@edani

This comment has been minimized.

Show comment Hide comment
@edani

edani Apr 29, 2014

Contributor

Most issues with CRLF files have been fixed in recent commits. ensime-refactor and ensime-search may be the only place where the problem remains.

Contributor

edani commented Apr 29, 2014

Most issues with CRLF files have been fixed in recent commits. ensime-refactor and ensime-search may be the only place where the problem remains.

@delexi

This comment has been minimized.

Show comment Hide comment
@delexi

delexi Apr 29, 2014

Ok, I didn't know that, sorry for the inconvenience.

delexi commented Apr 29, 2014

Ok, I didn't know that, sorry for the inconvenience.

@edani

This comment has been minimized.

Show comment Hide comment
@edani

edani Apr 29, 2014

Contributor

No problem, it's great to see activity pick up again!

Contributor

edani commented Apr 29, 2014

No problem, it's great to see activity pick up again!

@fommil fommil added this to the Backlog milestone Jun 17, 2014

@fommil fommil closed this Jun 22, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment