Re: Feedback on getting rid of `term-suppress-hard-newline'

[Stefan Monnier <monnier@iro.umontreal.ca>]

> ping

FWIW, I think this makes a lot of sense, so please go ahead.
A few comments below,


        Stefan


>> @@ -1106,6 +1106,7 @@ term-mode
>>    (make-local-variable 'term-scroll-show-maximum-output)
>>    (make-local-variable 'term-ptyp)
>>    (make-local-variable 'term-exec-hook)
>> +  (setq-local filter-buffer-substring-function 'term-filter-buffer-substring)

Please use `add-function` to modify filter-buffer-substring-function.

>> +    (put-text-property old-point (point) 'term-newline t)))

I'd recommend you use a more explicit name which doesn't just state the
obvious "this is a newline in term mode".  E.g. something like
`term-wrap-newline`.

>> +    (let (buffer-read-only)
>> +      (delete-char 1))))

Never let-bind `buffer-read-only`: let-bind `inhibit-read-only` instead.

>> +    ;; delete all fake newlines

Please capitalize and punctuate your comments.

>> +          (when (> len width)
>> +            (goto-char (+ bol width))

Not sure how well term deals with wide chars (e.g. TAB or chinese
chars), but this code of yours clearly won't help.  I think you
want to test current-column rather than `len` and then you want
move-to-column rather than advancing by N chars.