bug#26328: 26.0.50; checkdoc action for join lines drops final "

bug#26328: 26.0.50; checkdoc action for join lines drops final "

[Marco Wahl]

Checkdoc drops the final " when the action to join the lines has been
choosen.

E.g. have checkdoc enabled and eval

    (defun foo ()
    "bla bla
    bla."
    )

Suggestion for a fix:

modified   lisp/emacs-lisp/checkdoc.el
@@ -1520,7 +1520,7 @@ checkdoc-this-string-valid-engine
 			 ;; They said yes.  We have more fill work to do...
 			 (goto-char (match-beginning 1))
 			 (delete-region (point) (match-end 1))
-			 (insert "\n")
+			 (insert "\"")
 			 (setq msg nil))))))
 	   (if msg
 	       (checkdoc-create-error msg s (save-excursion



In GNU Emacs 26.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version 3.22.10)
 of 2017-03-29 built on tm6592
Repository revision: d707ba846902128572f420241897cbb966363ede
Windowing system distributor 'The X.Org Foundation', version 11.0.11903000
System Description:	Arch Linux

Recent messages:
Opening nndoc server on /home/b/Mail/archive/kusen...done
Opening nndoc server on /home/b/Mail/archive/todo...done
Opening nndoc server on /home/b/Mail/archive/vwiki...done
Reading active file via nndraft...done
Reading active file from gnorbthing via nnir...done
Checking new news...done
Mark saved where search started
Saving file /home/b/.newsrc-dribble...
Wrote /home/b/.newsrc-dribble [2 times]
(Saved .newsrc-dribble)

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GCONF GSETTINGS
NOTIFY ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB
TOOLKIT_SCROLL_BARS GTK3 X11 LIBSYSTEMD

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Magit

Minor modes in effect:
  beacon-mode: t
  moz-controller-global-mode: t
  moz-controller-mode: t
  zen-reward-mode: t
  nyan-mode: t
  global-company-mode: t
  company-mode: t
  diff-auto-refine-mode: t
  magit-auto-revert-mode: t
  global-git-commit-mode: t
  async-bytecomp-package-mode: t
  global-lentic-mode: t
  lentic-mode: t
  ido-hacks-mode: t
  ido-everywhere: t
  on-screen-global-mode: t
  wrap-region-global-mode: t
  wrap-region-mode: t
  keyfreq-autosave-mode: t
  keyfreq-mode: t
  dired-async-mode: t
  override-global-mode: t
  shell-dirtrack-mode: t
  timeclock-mode-line-display: t
  show-paren-mode: t
  erc-list-mode: t
  erc-menu-mode: t
  erc-autojoin-mode: t
  erc-ring-mode: t
  erc-networks-mode: t
  erc-pcomplete-mode: t
  erc-track-mode: t
  erc-track-minor-mode: t
  erc-match-mode: t
  erc-button-mode: t
  erc-fill-mode: t
  erc-stamp-mode: t
  erc-netsplit-mode: t
  erc-irccontrols-mode: t
  erc-noncommands-mode: t
  erc-move-to-prompt-mode: t
  erc-readonly-mode: t
  display-time-mode: t
  global-eldoc-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  buffer-read-only: t
  line-number-mode: t
  transient-mark-mode: t
  abbrev-mode: t

Load-path shadows:
Error during checking
Features:
(shadow bbdb-message mailalias emacsbug shr-color color flow-fill lyrics
follow paredit mc-cycle-cursors mc-mark-more mc-edit-lines caps-lock
eieio-opt speedbar sb-image ezimage dframe gnus-dired sh-script smie
executable zone zone-nyan esxml ace-window url-cache mm-archive smiley
gnus-cite gnus-bcklg mail-extr gnus-async gnus-kill qp gnus-ml nndraft
nnmh nndoc utf-7 network-stream starttls nnfolder bbdb-gnus nnnil
gnus-agent gnus-srvr gnus-score score-mode nnvirtual gnus-msg gnus-cache
gnus-demon nntp smtpmail sendmail cal-move bookmark tabify misearch
multi-isearch company-emoji company-emoji-list org-archive epa-file
password-store hl-line cal-iso org-duration beacon checkdoc view
page-ext erc-dcc battery cal-china lunar cal-islam cal-hebrew holidays
hol-loaddefs bbdb-anniv appt diary-lib diary-loaddefs emacsshot
rope-read-mode org-timer org-clock disp-table org-velocity dabbrev
org-learn org-invoice org-elisp-symbol org-depend org-w3m org-rmail
org-mhe org-irc org-inlinetask org-info org-habit org-gnus org-eww
org-docview doc-view jka-compr image-mode org-ctags org-crypt org-bibtex
bibtex org-bbdb exwm-randr xcb-randr exwm-config exwm exwm-input
xcb-keysyms xcb-xkb exwm-manage exwm-floating xcb-cursor xcb-render
exwm-layout exwm-workspace exwm-core xcb-ewmh xcb-icccm xcb xcb-xproto
xcb-types jl-encrypt maxima maxima-font-lock moz-controller moz
org-section-numbers org-structure-as-dirs-and-files org-bullets
carry-region cursor-color-mode ariadne-marks mw-mark txr-mode
auto-complete auxies-eww hacks auxies-rest org-supplements
little-helpers bbdb-mua bbdb-com gnorb-bbdb bbdb bbdb-site timezone
bbdb-loaddefs emms-librefm-stream emms-librefm-scrobbler
emms-playlist-limit emms-volume emms-volume-amixer emms-i18n
emms-history emms-score emms-stream-info emms-metaplaylist-mode
emms-bookmarks emms-cue emms-mode-line-icon emms-browser sort
emms-playlist-sort emms-last-played emms-player-xine emms-player-mpd tq
emms-playing-time emms-lyrics emms-url emms-streams emms-show-all
emms-tag-editor emms-mark emms-mode-line emms-cache emms-info-ogginfo
emms-info-mp3info emms-info later-do emms-playlist-mode emms-player-vlc
emms-player-mplayer emms-player-simple emms-source-playlist
emms-source-file locate emms-setup emms emms-compat org-protocol
slime-fancy slime-trace-dialog slime-fontifying-fu slime-package-fu
slime-references slime-compiler-notes-tree slime-scratch
slime-presentations bridge slime-mdot-fu slime-enclosing-context
slime-fuzzy slime-fancy-trace slime-fancy-inspector slime-c-p-c
slime-editing-commands slime-autodoc slime-repl elp slime-parse slime
gud apropos compile arc-mode archive-mode hyperspec slime-autoloads
refine git-timemachine elmacro emr-c emr-elisp emr-lisp list-utils
emr-iedit which-func imenu emr-prog emr popup git-auto-commit-mode
nyan-mode sotlisp skeleton gnuplot info-look helm helm-source
helm-multi-match helm-lib aurel url-http url-auth url-gw nsm bui
bui-list bui-info bui-entry bui-core bui-history bui-button bui-utils
cus-edit json map rase solar cal-dst gnorb gnorb-org gnorb-registry
gnus-registry registry gnorb-gnus gnorb-utils org-agenda org-capture
org-attach vc-git org-id gnus-art mm-uu mml2015 mm-view mml-smime smime
dig nngnorb nnir gnus-sum gnus-group gnus-undo gnus-start gnus-cloud
nnimap nnmail mail-source tls gnutls utf7 netrc gnus-spec gnus-int
gnus-range gnus-win nnoo smartparens lispy hydra lv swiper ivy
ivy-overlay ffap iedit iedit-lib multiple-cursors-core rect lispy-inline
semantic/db eieio-base semantic/util-modes semantic/util semantic
semantic/tag semantic/lex semantic/fw mode-local cedet ediff-merg
ediff-wind ediff-diff ediff-mult ediff-help ediff-init ediff-util ediff
edebug help-fns radix-tree lispy-tags key-chord company-oddmuse
company-keywords company-etags etags xref project company-gtags
company-dabbrev-code company-dabbrev company-files company-capf
company-cmake company-xcode company-clang company-semantic company-eclim
company-template company-css company-nxml company-bbdb company
magit-obsolete magit-blame magit-stash magit-bisect magit-remote
magit-commit magit-sequence magit-notes magit-worktree magit-branch
magit-files magit-refs magit-status magit magit-repos magit-apply
magit-wip magit-log magit-diff smerge-mode diff-mode magit-core
magit-autorevert autorevert filenotify magit-process magit-margin
magit-mode magit-git crm magit-section magit-popup git-commit
magit-utils log-edit message rfc822 mml mml-sec epa epg mm-decode
mm-bodies mm-encode mail-parse rfc2231 mailabbrev gmm-utils mailheader
pcvs-util add-log with-editor async-bytecomp tramp-sh server lentic-mode
lentic-doc lentic-ox lentic-org lentic-chunk rx ox-texinfo ox-org ox-odt
rng-loc rng-uri rng-parse rng-match rng-dt rng-util rng-pttrn nxml-parse
nxml-ns nxml-enc xmltok nxml-util ox-md ox-icalendar ox-html table
ox-beamer ox-latex ox-ascii ox-publish ox eww puny mm-url gnus nnheader
gnus-util rmail rmail-loaddefs rfc2047 rfc2045 ietf-drums mail-utils
mm-util mail-prsvr url-queue url url-proxy url-privacy url-expand
url-methods url-history url-cookie url-domsuf url-util mailcap shr svg
dom browse-url lentic eieio-compat f s m-buffer-at m-buffer
m-buffer-macro ido-hacks ido expand-region text-mode-expansions
cc-mode-expansions the-org-mode-expansions er-basic-expansions
expand-region-core expand-region-custom browse-kill-ring derived
form-feed page-break-lines on-screen wrap-region keyfreq stumpwm-mode
dired-narrow delsel dired-hacks-utils dash chronos notifications dbus
xml ace-link avy camcorder dired-async dired-aux dired dired-loaddefs
async use-package diminish bind-key finder-inf tex-site edmacro kmacro
screenshot-autoloads info package epg-config url-handlers url-parse
url-vars org-element avl-tree org org-macro org-footnote org-pcomplete
org-list org-faces org-entities noutline outline easy-mmode org-version
ob-octave ob-makefile ob-forth ob-R ob-haskell ob-maxima ob-java
ob-plantuml ob-sqlite ob-sql ob-screen ob-J ob-io ob-shell ob-clojure
ob-scheme ob-gnuplot ob-dot ob-ditaa ob-lisp ob-css ob-js ob-org
ob-ledger ob-latex ob-calc calc-store calc-trail calc-ext calc
calc-loaddefs calc-macs ob-C cc-mode cc-fonts cc-guess cc-menus cc-cmds
cc-styles cc-align cc-engine cc-vars cc-defs cl ob-python ob-awk
ob-emacs-lisp ob ob-tangle org-src ob-ref ob-lob ob-table ob-keys ob-exp
ob-comint tramp tramp-compat tramp-loaddefs trampver ucs-normalize shell
parse-time advice ob-core ob-eval org-compat org-macs org-loaddefs
cal-menu calendar cal-loaddefs ert find-func seq ewoc debug
.emacs-custom timeclock paren avoid erc-list erc-menu erc-join erc-ring
erc-networks erc-pcomplete time-date pcomplete comint ansi-color ring
erc-track erc-match erc-button wid-edit erc-fill erc-stamp erc-netsplit
erc-goodies erc erc-backend erc-compat format-spec auth-source cl-seq
eieio byte-opt subr-x bytecomp byte-compile cl-extra help-mode easymenu
cconv eieio-core cl-macs gv eieio-loaddefs password-cache thingatpt pp
time desktop frameset cl-loaddefs pcase cl-lib cus-start cus-load
mule-util tooltip eldoc electric uniquify ediff-hook vc-hooks
lisp-float-type mwheel term/x-win x-win term/common-win x-dnd tool-bar
dnd fontset image regexp-opt fringe tabulated-list replace newcomment
text-mode elisp-mode lisp-mode prog-mode register page menu-bar
rfn-eshadow isearch timer select scroll-bar mouse jit-lock font-lock
syntax facemenu font-core term/tty-colors frame cl-generic cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean
japanese eucjp-ms cp51932 hebrew greek romanian slovak czech european
ethiopic indian cyrillic chinese composite charscript case-table
epa-hook jka-cmpr-hook help simple abbrev obarray minibuffer
cl-preloaded nadvice loaddefs button faces cus-face macroexp files
text-properties overlay sha1 md5 base64 format env code-pages mule
custom widget hashtable-print-readable backquote dbusbind inotify
dynamic-setting system-font-setting font-render-setting move-toolbar gtk
x-toolkit x multi-tty make-network-process emacs)

Memory information:
((conses 16 1440895 235974)
 (symbols 48 95512 781)
 (miscs 40 64313 17720)
 (strings 32 370256 43439)
 (string-bytes 1 10722939)
 (vectors 16 155457)
 (vector-slots 8 3364869 113444)
 (floats 8 2015 3714)
 (intervals 56 19659 5441)
 (buffers 976 113))

-- 
Marco Wahl -- Freelancer
https://marcowahl.github.io

bug#26328: 26.0.50; checkdoc action for join lines drops final "

[Lars Ingebrigtsen]

Marco Wahl <marcowahlsoft@gmail.com> writes:

> Checkdoc drops the final " when the action to join the lines has been
> choosen.
>
> E.g. have checkdoc enabled and eval
>
>     (defun foo ()
>     "bla bla
>     bla."
>     )
>
> Suggestion for a fix:
>
> modified   lisp/emacs-lisp/checkdoc.el
> @@ -1520,7 +1520,7 @@ checkdoc-this-string-valid-engine
>  			 ;; They said yes.  We have more fill work to do...
>  			 (goto-char (match-beginning 1))
>  			 (delete-region (point) (match-end 1))
> -			 (insert "\n")
> +			 (insert "\"")
>  			 (setq msg nil))))))
>  	   (if msg
>  	       (checkdoc-create-error msg s (save-excursion

The " at the end of the doc string is removed by that `delete-region',
so it's all a bit confusing.  The following patch also fixes the problem
in this example, but I'm not quite sure what the code is attempting to
do here.

Anybody familiar with this code?

diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index 830743f5f8..7ac557711a 100644
--- a/lisp/emacs-lisp/checkdoc.el
+++ b/lisp/emacs-lisp/checkdoc.el
@@ -1498,12 +1498,7 @@ checkdoc-this-string-valid-engine
 			p (1+ p)
 			"1st line not a complete sentence.  Join these lines? "
 			" " t)
-		       (progn
-			 ;; They said yes.  We have more fill work to do...
-			 (goto-char (match-beginning 1))
-			 (delete-region (point) (match-end 1))
-			 (insert "\n")
-			 (setq msg nil))))))
+		       (setq msg nil)))))
 	   (if msg
 	       (checkdoc-create-error msg s (save-excursion
 					      (goto-char s)


-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#26328: 26.0.50; checkdoc action for join lines drops final "

[Noam Postavsky]

>> Suggestion for a fix:
>>
>> modified   lisp/emacs-lisp/checkdoc.el
>> @@ -1520,7 +1520,7 @@ checkdoc-this-string-valid-engine
>>  			 ;; They said yes.  We have more fill work to do...
>>  			 (goto-char (match-beginning 1))
>>  			 (delete-region (point) (match-end 1))
>> -			 (insert "\n")
>> +			 (insert "\"")
>>  			 (setq msg nil))))))
>>  	   (if msg
>>  	       (checkdoc-create-error msg s (save-excursion
>
> The " at the end of the doc string is removed by that `delete-region',
> so it's all a bit confusing.  The following patch also fixes the problem
> in this example, but I'm not quite sure what the code is attempting to
> do here.
>
> Anybody familiar with this code?
>
> diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
> index 830743f5f8..7ac557711a 100644
> --- a/lisp/emacs-lisp/checkdoc.el
> +++ b/lisp/emacs-lisp/checkdoc.el
> @@ -1498,12 +1498,7 @@ checkdoc-this-string-valid-engine
>  			p (1+ p)
>  			"1st line not a complete sentence.  Join these lines? "
>  			" " t)
> -		       (progn
> -			 ;; They said yes.  We have more fill work to do...
> -			 (goto-char (match-beginning 1))
> -			 (delete-region (point) (match-end 1))
> -			 (insert "\n")
> -			 (setq msg nil))))))
> +		       (setq msg nil)))))
>  	   (if msg
>  	       (checkdoc-create-error msg s (save-excursion
>  					      (goto-char s)

AFAICT, both proposed patches will do the wrong thing for this case:

    (defun foo ()
      "Bla bla
    bla bla.  More words
    bla bla bla.")

bug#26328: 26.0.50; checkdoc action for join lines drops final "

[Lars Ingebrigtsen]

Noam Postavsky <npostavs@gmail.com> writes:

> AFAICT, both proposed patches will do the wrong thing for this case:
>
>     (defun foo ()
>       "Bla bla
>     bla bla.  More words
>     bla bla bla.")

Yup.  It's rather unclear what that code is attempting to do...

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#26328: 26.0.50; checkdoc action for join lines drops final "

[Alex Branham]

[-- Attachment #1: Type: text/plain, Size: 529 bytes --]

tags 26328 + patch
quit

On Mon 29 Jul 2019 at 13:09, Lars Ingebrigtsen <larsi@gnus.org> wrote:

> Noam Postavsky <npostavs@gmail.com> writes:
>
>> AFAICT, both proposed patches will do the wrong thing for this case:
>>
>>     (defun foo ()
>>       "Bla bla
>>     bla bla.  More words
>>     bla bla bla.")
>
> Yup.  It's rather unclear what that code is attempting to do...

How about we just use delete-indentation along with the
already-calculate position p that we just asked the user about? Patch
attached.

Thanks,
Alex


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-checkdoc-incorrectly-joining-lines.patch --]
[-- Type: text/x-patch, Size: 1515 bytes --]

From 41c09cedd36cea604bd6a1ea20dafdc44ce31da0 Mon Sep 17 00:00:00 2001
From: Alex Branham <alex.branham@gmail.com>
Date: Thu, 15 Aug 2019 18:04:33 -0500
Subject: [PATCH] Fix checkdoc incorrectly joining lines

* lisp/emacs-lisp/checkdoc.el (checkdoc-this-string-valid-engine):
Use delete-indentation rather than delete-region.

Bug#26328
---
 lisp/emacs-lisp/checkdoc.el | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index 830743f5f8..9e7291ba21 100644
--- a/lisp/emacs-lisp/checkdoc.el
+++ b/lisp/emacs-lisp/checkdoc.el
@@ -1494,16 +1494,13 @@ checkdoc-this-string-valid-engine
 	       (if (and (re-search-forward "[.!?:\"]\\([ \t\n]+\\|\"\\)"
 					   (line-end-position) t)
 			(< (current-column) numc))
-		   (if (checkdoc-autofix-ask-replace
-			p (1+ p)
-			"1st line not a complete sentence.  Join these lines? "
-			" " t)
-		       (progn
-			 ;; They said yes.  We have more fill work to do...
-			 (goto-char (match-beginning 1))
-			 (delete-region (point) (match-end 1))
-			 (insert "\n")
-			 (setq msg nil))))))
+		   (when (checkdoc-autofix-ask-replace
+		          p (1+ p)
+		          "1st line not a complete sentence.  Join these lines? "
+		          " " t)
+		     ;; They said yes.  We have more fill work to do...
+		     (delete-indentation nil p (1+ p))
+		     (setq msg nil)))))
 	   (if msg
 	       (checkdoc-create-error msg s (save-excursion
 					      (goto-char s)
-- 
2.22.0

bug#26328: 26.0.50; checkdoc action for join lines drops final "

[Lars Ingebrigtsen]

Alex Branham <alex.branham@gmail.com> writes:

>>> AFAICT, both proposed patches will do the wrong thing for this case:
>>>
>>>     (defun foo ()
>>>       "Bla bla
>>>     bla bla.  More words
>>>     bla bla bla.")

[...]

> +		   (when (checkdoc-autofix-ask-replace
> +		          p (1+ p)
> +		          "1st line not a complete sentence.  Join these lines? "
> +		          " " t)
> +		     ;; They said yes.  We have more fill work to do...
> +		     (delete-indentation nil p (1+ p))
> +		     (setq msg nil)))))

This looks like the right thing for the original use case, but will it
fix the example above?

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#26328: 26.0.50; checkdoc action for join lines drops final "

[Noam Postavsky]

Alex Branham <alex.branham@gmail.com> writes:

> How about we just use delete-indentation along with the
> already-calculate position p that we just asked the user about? Patch
> attached.

> +		   (when (checkdoc-autofix-ask-replace
> +		          p (1+ p)
> +		          "1st line not a complete sentence.  Join these lines? "
> +		          " " t)
> +		     ;; They said yes.  We have more fill work to do...
> +		     (delete-indentation nil p (1+ p))
> +		     (setq msg nil)))))

Note that the checkdoc-autofix-ask-replace is already replacing the
newline with a space (which is kind of a strange behaviour, IMO, but
that's how it is currently), so I'm not sure it makes much sense to call
delete-indentation after.

bug#26328: 26.0.50; checkdoc action for join lines drops final "

[Alex Branham]

[-- Attachment #1: Type: text/plain, Size: 753 bytes --]

On Thu 15 Aug 2019 at 20:11, Noam Postavsky <npostavs@gmail.com> wrote:

> Alex Branham <alex.branham@gmail.com> writes:
>
> Note that the checkdoc-autofix-ask-replace is already replacing the
> newline with a space (which is kind of a strange behaviour, IMO, but
> that's how it is currently), so I'm not sure it makes much sense to call
> delete-indentation after.

Ah, indeed. Starting with this buffer:

(defun foo ()
  "bla bla
    bla."
  )

(defun foo ()
  "Bla bla
    bla bla.  More words
    bla bla bla.")

If we just remove the call to delete-indentation we wind up with:

(defun foo ()
  "Bla bla bla."
  )

(defun foo ()
  "Bla bla bla bla.
More words bla bla bla.")

So perhaps the we just remove that bit? Updated patch attached.

Alex


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Avoid-deleting-closing-quotation-mark-in-checkdoc.patch --]
[-- Type: text/x-patch, Size: 1405 bytes --]

From b2f2ef1731c28d0f541c7778dd7a49774f59ddb7 Mon Sep 17 00:00:00 2001
From: Alex Branham <alex.branham@gmail.com>
Date: Fri, 16 Aug 2019 08:22:23 -0500
Subject: [PATCH] Avoid deleting closing quotation mark in checkdoc

* lisp/emacs-lisp/checkdoc.el (checkdoc-this-string-valid-engine):
Remove calls to delete-region (bug#26328).
---
 lisp/emacs-lisp/checkdoc.el | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/lisp/emacs-lisp/checkdoc.el b/lisp/emacs-lisp/checkdoc.el
index 830743f5f8..3c69975021 100644
--- a/lisp/emacs-lisp/checkdoc.el
+++ b/lisp/emacs-lisp/checkdoc.el
@@ -1494,16 +1494,11 @@ checkdoc-this-string-valid-engine
 	       (if (and (re-search-forward "[.!?:\"]\\([ \t\n]+\\|\"\\)"
 					   (line-end-position) t)
 			(< (current-column) numc))
-		   (if (checkdoc-autofix-ask-replace
-			p (1+ p)
-			"1st line not a complete sentence.  Join these lines? "
-			" " t)
-		       (progn
-			 ;; They said yes.  We have more fill work to do...
-			 (goto-char (match-beginning 1))
-			 (delete-region (point) (match-end 1))
-			 (insert "\n")
-			 (setq msg nil))))))
+		   (when (checkdoc-autofix-ask-replace
+		          p (1+ p)
+		          "1st line not a complete sentence.  Join these lines? "
+		          " " t)
+		     (setq msg nil)))))
 	   (if msg
 	       (checkdoc-create-error msg s (save-excursion
 					      (goto-char s)
-- 
2.22.0

bug#26328: 26.0.50; checkdoc action for join lines drops final "

[Noam Postavsky]

Alex Branham <alex.branham@gmail.com> writes:

> On Thu 15 Aug 2019 at 20:11, Noam Postavsky <npostavs@gmail.com> wrote:
>
>> Note that the checkdoc-autofix-ask-replace is already replacing the
>> newline with a space (which is kind of a strange behaviour, IMO, but
>> that's how it is currently), so I'm not sure it makes much sense to call
>> delete-indentation after.
>
> Ah, indeed. Starting with this buffer:
>
> (defun foo ()
>   "bla bla
>     bla."
>   )
>
> (defun foo ()
>   "Bla bla
>     bla bla.  More words
>     bla bla bla.")
>
> If we just remove the call to delete-indentation we wind up with:
>
> (defun foo ()
>   "Bla bla bla."
>   )
>
> (defun foo ()
>   "Bla bla bla bla.
> More words bla bla bla.")
>
> So perhaps the we just remove that bit? Updated patch attached.

> +		   (when (checkdoc-autofix-ask-replace
> +		          p (1+ p)
> +		          "1st line not a complete sentence.  Join these lines? "
> +		          " " t)
> +		     (setq msg nil)))))

This is like Lars' suggestion, and using this I get

(defun foo ()
  "Bla bla bla bla.  More words
    bla bla bla.")

for the second case which is incorrect, I think.

bug#26328: 26.0.50; checkdoc action for join lines drops final "

[Alex Branham]

On Fri 16 Aug 2019 at 09:42, Noam Postavsky <npostavs@gmail.com> wrote:

> Alex Branham <alex.branham@gmail.com> writes:
> This is like Lars' suggestion, and using this I get
>
> (defun foo ()
>   "Bla bla bla bla.  More words
>     bla bla bla.")
>
> for the second case which is incorrect, I think.

Right, which is caught later in checkdoc and fixed. I don't think asking
users to push "f" twice is a big deal. An alternative could be to join
the lines, then insert a newline after the end of the sentence, then run
fill-pararaph?

Alex

bug#26328: 26.0.50; checkdoc action for join lines drops final "

[Noam Postavsky]

Alex Branham <alex.branham@gmail.com> writes:

> On Fri 16 Aug 2019 at 09:42, Noam Postavsky <npostavs@gmail.com> wrote:
>
>> This is like Lars' suggestion, and using this I get
>>
>> (defun foo ()
>>   "Bla bla bla bla.  More words
>>     bla bla bla.")
>>
>> for the second case which is incorrect, I think.
>
> Right, which is caught later in checkdoc and fixed. I don't think asking
> users to push "f" twice is a big deal.

Ah, I was just calling M-x checkdoc-defun, which leaves it at the above.
If users of checkdoc think it's okay to do it like that, then it's fine
by me.

bug#26328: 26.0.50; checkdoc action for join lines drops final "

[Lars Ingebrigtsen]

Alex Branham <alex.branham@gmail.com> writes:

> Remove calls to delete-region (bug#26328).

Given the two test cases, this makes checkdoc work for me, and I'm
applying the patch.

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no