emacsorphanage / helm-ag

The silver searcher with helm interface
492 stars 76 forks source link

helm-ag-edit duplicates carriage returns when using linux rg/ag on CRLF files #384

Open mosquito-magnet opened 3 years ago

mosquito-magnet commented 3 years ago

Actual behavior

When using ripgrep or ag from linux with helm-ag, and matching files that contain CR-LF (windows-style) line endings, and running helm-ag-edit on them, the last carriage return (CR) will be duplicated on commit.

Expected behavior

Carriage return is not duplicated.

Steps to reproduce

Analysis

The matches from ag and rg on linux against files with CR-LF line endings will contain a trailing CR, so helm-ag and helm-ag-edit will treat it as part of the line content. When helm-ag--edit-commit is called, the file's encoding including line ending style is properly recognized by insert-file-content - for CR-LF files this means that the code for clearing the original line content ((delete-region (line-beginning-position) (line-end-position))) will leave the CR of the original line intact. Now the potentially edited line content from the edit buffer with the trailing CR is inserted, and we end up with CR-CR-LF at the end.

The scenario emacs and rg/ag running on Windows works fine. Not sure if that's creditable to the way shell output is read, or rg/ag.

I suppose this isn't really a helm-ag bug, but a consequence of ag and rg not trying to be smart about line ending style (they should be fast after all). Not sure what to make of this, I mainly post it here for others to find. Maybe add it to readme or wiki? So far I'm content with the following workaround:

Workaround

Only applies when running rg/ag on Linux, with the Windows versions of rg/ag it would mess up line endings (CR-LF to LF).

(defun helm-ag--edit-commit-adv-no-conversion (old-fun &rest args)
  (let ((coding-system-for-read 'no-conversion))
    (apply old-fun args)))
(advice-add 'helm-ag--edit-commit :around #'helm-ag--edit-commit-adv-no-conversion)