all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Re: master 78fc49407b8 1/3: Improve filling of ChangeLog entries
       [not found] ` <20240128085846.187A2C1DAE4@vcs2.savannah.gnu.org>
@ 2024-01-28 13:22   ` Dmitry Gutov
  2024-01-28 13:37     ` Po Lu
  2024-01-30 22:07     ` João Távora
  2024-01-29 20:13   ` Stefan Kangas
  1 sibling, 2 replies; 18+ messages in thread
From: Dmitry Gutov @ 2024-01-28 13:22 UTC (permalink / raw)
  To: emacs-devel, Po Lu

On 28/01/2024 10:58, Po Lu via Mailing list for Emacs changes wrote:
> (log-edit-fill-entry): Abandon pcase and cl-lib.

That seems like a gratuitous change.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: master 78fc49407b8 1/3: Improve filling of ChangeLog entries
  2024-01-28 13:22   ` master 78fc49407b8 1/3: Improve filling of ChangeLog entries Dmitry Gutov
@ 2024-01-28 13:37     ` Po Lu
  2024-01-30 22:07     ` João Távora
  1 sibling, 0 replies; 18+ messages in thread
From: Po Lu @ 2024-01-28 13:37 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

Dmitry Gutov <dmitry@gutov.dev> writes:

> That seems like a gratuitous change.

It is not.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: master 78fc49407b8 1/3: Improve filling of ChangeLog entries
       [not found] ` <20240128085846.187A2C1DAE4@vcs2.savannah.gnu.org>
  2024-01-28 13:22   ` master 78fc49407b8 1/3: Improve filling of ChangeLog entries Dmitry Gutov
@ 2024-01-29 20:13   ` Stefan Kangas
  2024-01-30  1:26     ` Po Lu
  1 sibling, 1 reply; 18+ messages in thread
From: Stefan Kangas @ 2024-01-29 20:13 UTC (permalink / raw)
  To: Po Lu, emacs-devel

Po Lu via Mailing list for Emacs changes <emacs-diffs@gnu.org> writes:

> branch: master
> commit 78fc49407b8ef8ec649fe70fcce09101801dbc05
> Author: Po Lu <luangruo@yahoo.com>
> Commit: Po Lu <luangruo@yahoo.com>
>
>     Improve filling of ChangeLog entries
>
>     * lisp/vc/log-edit.el (log-edit--insert-filled-defuns): Rewrite
>     completely.
>     (log-edit-fill-entry): Abandon pcase and cl-lib.

Speaking of commit messages, the above is not at all clear to me.
More specifically, the word "improve" seems open to interpretation.

Is it just a rewrite to "abandon pcase and cl-lib"?  If yes, I think
that is what should have gone on the first line summary.

If that's not it, what is it that has been improved?



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: master 78fc49407b8 1/3: Improve filling of ChangeLog entries
  2024-01-29 20:13   ` Stefan Kangas
@ 2024-01-30  1:26     ` Po Lu
  0 siblings, 0 replies; 18+ messages in thread
From: Po Lu @ 2024-01-30  1:26 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: emacs-devel

Stefan Kangas <stefankangas@gmail.com> writes:

> Po Lu via Mailing list for Emacs changes <emacs-diffs@gnu.org> writes:
>
>> branch: master
>> commit 78fc49407b8ef8ec649fe70fcce09101801dbc05
>> Author: Po Lu <luangruo@yahoo.com>
>> Commit: Po Lu <luangruo@yahoo.com>
>>
>>     Improve filling of ChangeLog entries
>>
>>     * lisp/vc/log-edit.el (log-edit--insert-filled-defuns): Rewrite
>>     completely.
>>     (log-edit-fill-entry): Abandon pcase and cl-lib.
>
> Speaking of commit messages, the above is not at all clear to me.
> More specifically, the word "improve" seems open to interpretation.
>
> Is it just a rewrite to "abandon pcase and cl-lib"?  If yes, I think
> that is what should have gone on the first line summary.
>
> If that's not it, what is it that has been improved?

Several bugs causing entries to be improperly filled have been fixed,
and probably countless more I was unaware of; with fill-column set to
44, for example,

* lisp/progmodes/cmake-ts-mode.el 
(treesit-induce-sparse-tree)
(treesit-node-child, treesit-node-start)
(cmake-ts-mode--imenu)
(cmake-ts-mode--imenu-1): Remove.
(treesit-search-subtree): Declare.
(cmake-ts-mode--function-name): New
function.
(cmake-ts-mode): Use it.

would have been filled to:

* lisp/progmodes/cmake-ts-mode.el 
(treesit-induce-sparse-tree)
(treesit-node-child, treesit-node-start)
(cmake-ts-mode--imenu,
cmake-ts-mode--imenu-1): Remove.
(treesit-search-subtree): Declare.
(cmake-ts-mode--function-name): New
function.
(cmake-ts-mode): Use it.

Not understanding the original code, it was not possible for me to
describe the change in greater detail.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: master 78fc49407b8 1/3: Improve filling of ChangeLog entries
  2024-01-28 13:22   ` master 78fc49407b8 1/3: Improve filling of ChangeLog entries Dmitry Gutov
  2024-01-28 13:37     ` Po Lu
@ 2024-01-30 22:07     ` João Távora
  2024-01-31  6:43       ` Po Lu
  1 sibling, 1 reply; 18+ messages in thread
From: João Távora @ 2024-01-30 22:07 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: emacs-devel

Dmitry Gutov <dmitry@gutov.dev> writes:

> On 28/01/2024 10:58, Po Lu via Mailing list for Emacs changes wrote:
>> (log-edit-fill-entry): Abandon pcase and cl-lib.
>
> That seems like a gratuitous change.

Seems like this much simpler patch would have done the job.  If cl-loop
is so toxic, even macroexpanding these trivial ones away yields much
less code and much simpler code.  Tests are a welcome addition though.

João

diff --git a/lisp/vc/log-edit.el b/lisp/vc/log-edit.el
index 72867f14d2f..0ce21c5c473 100644
--- a/lisp/vc/log-edit.el
+++ b/lisp/vc/log-edit.el
@@ -578,15 +578,17 @@ log-edit--insert-filled-defuns
     (unless (or (memq (char-before) '(?\n ?\s))
                 (> (current-column) fill-column))
       (insert " "))
-    (cl-loop for first-fun = t then nil
-             for def in func-names do
-             (when (> (+ (current-column) (string-width def)) fill-column)
-               (unless first-fun
-                 (insert ")"))
-               (insert "\n"))
-             (insert (if (memq (char-before) '(?\n ?\s))
-                         "(" ", ")
-                     def))
+    (cl-loop with first-fun = t
+             for (def . more) on func-names do
+             (when (> (+ (current-column) (string-width def)
+                         (if first-fun 1 2)
+                         (if (null more) 1 0))
+                      fill-column)
+               (unless first-fun (insert ")"))
+               (unless (eq (char-before) ?\n) (insert "\n"))
+               (setq first-fun t))
+             (insert (if first-fun "(" ", ") def)
+             (setq first-fun nil))
     (insert "):")))
 
 (defun log-edit-fill-entry (&optional justify)
@@ -611,10 +613,11 @@ log-edit-fill-entry
               (copy-marker (match-end 1)))
          ;; Fill prose between log entries.
          do (let ((fill-indent-according-to-mode t)
-                  (end (if defuns-beg (match-beginning 0) end))
-                  (beg (progn (goto-char beg) (line-beginning-position))))
+                  (end (if defuns-beg (match-beginning 0) end)))
+              (goto-char beg)
+              (skip-chars-backward "^ \t\n")
               (when (<= (line-end-position) end)
-                (fill-region beg end justify)))
+                (fill-region (point) end justify)))
          while defuns-beg
          for defuns = (progn (goto-char defuns-beg)
                              (change-log-read-defuns end))





^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: master 78fc49407b8 1/3: Improve filling of ChangeLog entries
  2024-01-30 22:07     ` João Távora
@ 2024-01-31  6:43       ` Po Lu
  2024-01-31 10:28         ` João Távora
  0 siblings, 1 reply; 18+ messages in thread
From: Po Lu @ 2024-01-31  6:43 UTC (permalink / raw)
  To: João Távora; +Cc: Dmitry Gutov, emacs-devel

João Távora <joaotavora@gmail.com> writes:

> Seems like this much simpler patch would have done the job.  If cl-loop
> is so toxic, even macroexpanding these trivial ones away yields much
> less code and much simpler code.  Tests are a welcome addition though.

I did not understand the code using cl-loop, and there are no comments
in this version of the change.  For that matter, if there _were_
comments in the original version, I could have understood its intent,
but cryptic cl-lib and pcase labyrinths are all the rage these days.

Anyway, please don't modify log-edit just yet--I plan to modify it to
handle conditional changes and part indicators.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: master 78fc49407b8 1/3: Improve filling of ChangeLog entries
  2024-01-31  6:43       ` Po Lu
@ 2024-01-31 10:28         ` João Távora
  2024-01-31 11:19           ` Po Lu
  0 siblings, 1 reply; 18+ messages in thread
From: João Távora @ 2024-01-31 10:28 UTC (permalink / raw)
  To: Po Lu; +Cc: Dmitry Gutov, emacs-devel

On Wed, Jan 31, 2024 at 6:44 AM Po Lu <luangruo@yahoo.com> wrote:
>
> João Távora <joaotavora@gmail.com> writes:
>
> > Seems like this much simpler patch would have done the job.  If cl-loop
> > is so toxic, even macroexpanding these trivial ones away yields much
> > less code and much simpler code.  Tests are a welcome addition though.
>
> I did not understand the code using cl-loop,

Neither did I.  Not because of cl-loop, of course, but because of
the new problem domain.  But a few runs of edebug quickly clarified,
even before finding all those nice tests.

I think you're too obsessed with cl-loop. pp-macroexpand-last-sexp
would have also have told you how trivial the loops were.  You could
still have rewritten things with plain 'while' and still have it
be significantly shorter.

> and there are no comments in this version of the change.

Concise code not spanning multiple screenfuls doesn't need all those
profuse comments either.  A little commenting is fine (maybe a comment
explaining the sum of lengths) but the multiple screenfuls and
the level of verbosity you picked here makes a relatively easy problem
domain seem esoteric and super-complex, when in reality it isn't.

In my experience, once a function reaches this hairiness, it rarely
contracts.  All you'll see if more if-branches appear at the leaves.

> For that matter, if there _were_
> comments in the original version, I could have understood its intent,
> but cryptic cl-lib and pcase labyrinths are all the rage these days.

loop isn't fashion, it's probably older than you are, and not the beast you
make of it, especially these trivial cases.  I'm pretty sure you'd be able
to grok it in less than 5 minutes.

The pcase you substituted was even more trivial (really, "labyrith"??  how
extreme can you get?) Personally I wouldn't have added it, but of course
I wouldn't have removed it for no reason.  It's a hard sell that so much
of this doesn't spring from the ridiculous polarized trenches people
have been digging themselves into in this list.  This seems to me the
saddest part.

Honestly, Po, at the end of the day, we should all be happy you're fixing
bugs and adding unit tests.  You've got exceptional energy or exceptional
free time, or both.  I think freedoms should be given liberally to people
doing this work, including of course "rewriting the whole damn thing" (tm).
And of course you should have your own pick of the programming style.

Personally, I just wish you'd appreciate that high-level languages aren't
the bogey man.  That others might be put off by long listings (maybe even
future you, who knows?).  That rewrites spawn bugs, too (as your multiple
subsequent commits  show). That sometimes this:

-                  (beg (progn (goto-char beg) (line-beginning-position))))
+                  (beg (progn (goto-char beg) (skip-chars-backward "^
\n") (point))))

could be preferable to going medieval on a function's ass

> Anyway, please don't modify log-edit just yet--I plan to modify it to
> handle conditional changes and part indicators.

I won't, of course.  I'll leave it to you.

Also I'll be leaving emacs development soon, so you'll be able to
butcher Eglot's cl-loops and pcases and macros into whatever you see fit,
just try not to go too medieval on its ass.

João



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: master 78fc49407b8 1/3: Improve filling of ChangeLog entries
  2024-01-31 10:28         ` João Távora
@ 2024-01-31 11:19           ` Po Lu
  2024-01-31 13:22             ` João Távora
  0 siblings, 1 reply; 18+ messages in thread
From: Po Lu @ 2024-01-31 11:19 UTC (permalink / raw)
  To: João Távora; +Cc: Dmitry Gutov, emacs-devel

Joao, the diatribe was unnecessary (and I'm not certain I disagree with
most of it), since there's a world of difference between our two
perspectives.  I don't doubt that these particular `cl-loop' or `pcase'
forms are simple to understand, provided that the reader takes the ten
or so minutes to absorb the relevant portions of their documentation.

My issue with such forms is that spending those ten minutes every time I
fix a bug is incompatible with my workflow of choice for mostly
avocational projects such as Emacs, where I make a mental note of tasks
that interest me, and pick the least time-consuming of them to complete
whenever free time (which, surprising as it may be, is scarce) does come
my way.

That being my MO, it is most convenient for me to rewrite such functions
in a straightforward fashion rather than for elegance, or macroexpand
them as I did in parts of this change, and since the important metric at
the end of the day is the correctness of the resulting code, it is
harmless for that code to be more verbose than strictly required.

Which is also the reason I requested no more than that you not install
your version now, leaving the door open to future improvement on your
terms once I finish making it work.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: master 78fc49407b8 1/3: Improve filling of ChangeLog entries
  2024-01-31 11:19           ` Po Lu
@ 2024-01-31 13:22             ` João Távora
  2024-01-31 14:01               ` Po Lu
  0 siblings, 1 reply; 18+ messages in thread
From: João Távora @ 2024-01-31 13:22 UTC (permalink / raw)
  To: Po Lu; +Cc: Dmitry Gutov, emacs-devel

On Wed, Jan 31, 2024 at 11:19 AM Po Lu <luangruo@yahoo.com> wrote:

>  I don't doubt that these particular `cl-loop' or `pcase'
> forms are simple to understand, provided that the reader takes the ten
> or so minutes to absorb the relevant portions of their documentation.

As I said, I'd just use edebug or macroexpand these things away, like
I did.  You do know edebug, right?  And pp-macroexpand?  The latter
takes not even 10 seconds, what 10 minutes!??

Also, do you have any remote idea how much time it will take someone
to read your code which has grown those functions 3 or 4 fold??

> avocational projects such as Emacs, where I make a mental note of tasks
> that interest me, and pick the least time-consuming of them to complete
> whenever free time (which, surprising as it may be, is scarce) does come
> my way.

Judging by the number of follow-up bugfix commits, you misjudged that
badly.

> Which is also the reason I requested no more than that you not install
> your version now, leaving the door open to future improvement on your
> terms once I finish making it work.

Of course not, it's now yours.  Probably forever.

The unit tests are basically the only thing I can read from
your code.  Really, in practice you'll be probably be the only one ever
reading that long-winded Fotran-like code, so I wish you stick around
for a long time maintaining it.

The only "improvement" I'd have is reverting all your work and
applying my succinct patch that meets the current goalposts.
If you move them or invent new ones that's another matter... hopefully
that will also come with tests, or even some minimal discussion.

So while the code would go back to being simpler (even without cl-loop)
 you'd probably be demotivated to keep maintaing the file.  Also
the history is already botched, I'd just make it worse.  Really doesn't
justify rewriting your code. It'd be a poor, almost ridiculous, way to
"collaborate".

> diatribe was unnecessary (and I'm not certain I disagree with
> most of it)

If, in all that sea of bravado, it has dropped a grain of self-doubt
for next time you're pulling out the bulldozer, I consider it a diatribe
well tribed.

Anyway, have fun, and all the best
João



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: master 78fc49407b8 1/3: Improve filling of ChangeLog entries
  2024-01-31 13:22             ` João Távora
@ 2024-01-31 14:01               ` Po Lu
  2024-01-31 14:30                 ` João Távora
  2024-01-31 15:15                 ` Dmitry Gutov
  0 siblings, 2 replies; 18+ messages in thread
From: Po Lu @ 2024-01-31 14:01 UTC (permalink / raw)
  To: João Távora; +Cc: Dmitry Gutov, emacs-devel

João Távora <joaotavora@gmail.com> writes:

> The unit tests are basically the only thing I can read from
> your code.  Really, in practice you'll be probably be the only one ever
> reading that long-winded Fotran-like code, so I wish you stick around
> for a long time maintaining it.

Not only are the insults uncalled-for, the assumptions they rest on are
also completely untrue.  Great volumes of "long-winded Fortran-like
code" exist in Emacs with no shortage of maintainers, just as they do in
other established free software projects.  See any file in CC Mode, or
GCC's reload.cc, which features this massive conditional dwarfing any of
ours:

  scalar_int_mode inner_mode;
  if (in != 0 && GET_CODE (in) == SUBREG
      && targetm.can_change_mode_class (GET_MODE (SUBREG_REG (in)),
					inmode, rclass)
      && contains_allocatable_reg_of_mode[rclass][GET_MODE (SUBREG_REG (in))]
      && (strict_low
	  || (subreg_lowpart_p (in)
	      && (CONSTANT_P (SUBREG_REG (in))
		  || GET_CODE (SUBREG_REG (in)) == PLUS
		  || (((REG_P (SUBREG_REG (in))
			&& REGNO (SUBREG_REG (in)) >= FIRST_PSEUDO_REGISTER)
		       || MEM_P (SUBREG_REG (in)))
		      && (paradoxical_subreg_p (inmode,
						GET_MODE (SUBREG_REG (in)))
			  || (known_le (GET_MODE_SIZE (inmode), UNITS_PER_WORD)
			      && is_a <scalar_int_mode> (GET_MODE (SUBREG_REG
								   (in)),
							 &inner_mode)
			      && GET_MODE_SIZE (inner_mode) <= UNITS_PER_WORD
			      && paradoxical_subreg_p (inmode, inner_mode)
			      && LOAD_EXTEND_OP (inner_mode) != UNKNOWN)
			  || (WORD_REGISTER_OPERATIONS
			      && partial_subreg_p (inmode,
						   GET_MODE (SUBREG_REG (in)))
			      && (known_equal_after_align_down
				  (GET_MODE_SIZE (inmode) - 1,
				   GET_MODE_SIZE (GET_MODE (SUBREG_REG
							    (in))) - 1,
				   UNITS_PER_WORD)))))
		  || (REG_P (SUBREG_REG (in))
		      && REGNO (SUBREG_REG (in)) < FIRST_PSEUDO_REGISTER
		      /* The case where out is nonzero
			 is handled differently in the following statement.  */
		      && (out == 0 || subreg_lowpart_p (in))
		      && (complex_word_subreg_p (inmode, SUBREG_REG (in))
			  || !targetm.hard_regno_mode_ok (subreg_regno (in),
							  inmode)))
		  || (secondary_reload_class (1, rclass, inmode, in) != NO_REGS
		      && (secondary_reload_class (1, rclass,
						  GET_MODE (SUBREG_REG (in)),
						  SUBREG_REG (in))
			  == NO_REGS))
		  || (REG_P (SUBREG_REG (in))
		      && REGNO (SUBREG_REG (in)) < FIRST_PSEUDO_REGISTER
		      && !REG_CAN_CHANGE_MODE_P (REGNO (SUBREG_REG (in)),
						 GET_MODE (SUBREG_REG (in)),
						 inmode))))
	  || (REG_P (SUBREG_REG (in))
	      && REGNO (SUBREG_REG (in)) >= FIRST_PSEUDO_REGISTER
	      && reg_equiv_mem (REGNO (SUBREG_REG (in)))
	      && (mode_dependent_address_p
		  (XEXP (reg_equiv_mem (REGNO (SUBREG_REG (in))), 0),
		   MEM_ADDR_SPACE (reg_equiv_mem (REGNO (SUBREG_REG (in)))))))))

I'm aware that GCC is abandoning reload for a new register allocator,
whose coding style is not substantially different, but offers fewer in
the way of prodigious conditionals to prove my point.

Whether you want to work on log-edit.el, of course, is up to you.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: master 78fc49407b8 1/3: Improve filling of ChangeLog entries
  2024-01-31 14:01               ` Po Lu
@ 2024-01-31 14:30                 ` João Távora
  2024-01-31 15:15                 ` Dmitry Gutov
  1 sibling, 0 replies; 18+ messages in thread
From: João Távora @ 2024-01-31 14:30 UTC (permalink / raw)
  To: Po Lu; +Cc: Dmitry Gutov, emacs-devel

On Wed, Jan 31, 2024 at 2:01 PM Po Lu <luangruo@yahoo.com> wrote:

> See any file in CC Mode,

I have.  Glad I don't have to anymore :-)

> GCC's reload.cc, which features this massive conditional dwarfing any of
> ours:

Ooof indeed.  Not sure it's the best standard to have around for Lisp
though.

João



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: master 78fc49407b8 1/3: Improve filling of ChangeLog entries
  2024-01-31 14:01               ` Po Lu
  2024-01-31 14:30                 ` João Távora
@ 2024-01-31 15:15                 ` Dmitry Gutov
  2024-01-31 15:32                   ` Alan Mackenzie
  1 sibling, 1 reply; 18+ messages in thread
From: Dmitry Gutov @ 2024-01-31 15:15 UTC (permalink / raw)
  To: Po Lu, João Távora; +Cc: emacs-devel

On 31/01/2024 16:01, Po Lu wrote:
> See any file in CC Mode,

No shortage of maintainers, you say?

> or
> GCC's reload.cc, which features this massive conditional dwarfing any of
> ours:

Was there a way to write it more succinctly, using some higher-level 
constructs? That is the subject.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: master 78fc49407b8 1/3: Improve filling of ChangeLog entries
  2024-01-31 15:15                 ` Dmitry Gutov
@ 2024-01-31 15:32                   ` Alan Mackenzie
  2024-01-31 16:46                     ` João Távora
  2024-01-31 17:05                     ` Dmitry Gutov
  0 siblings, 2 replies; 18+ messages in thread
From: Alan Mackenzie @ 2024-01-31 15:32 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Po Lu, João Távora, emacs-devel

Hello, Dmitry.

On Wed, Jan 31, 2024 at 17:15:41 +0200, Dmitry Gutov wrote:
> On 31/01/2024 16:01, Po Lu wrote:
> > See any file in CC Mode,

> No shortage of maintainers, you say?

None.

> > or GCC's reload.cc, which features this massive conditional dwarfing
> > any of ours:

> Was there a way to write it more succinctly, using some higher-level 
> constructs? That is the subject.

I think the discussion is over the advantages and disadvantages of
replacing obscure concise code with its equivalent in plain Lisp.

After several days of struggling with named-let, cl-labels, and friends,
I vote for the plain Lisp, even if it does need more lines to express.
It is simply less work.

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: master 78fc49407b8 1/3: Improve filling of ChangeLog entries
  2024-01-31 15:32                   ` Alan Mackenzie
@ 2024-01-31 16:46                     ` João Távora
  2024-01-31 18:29                       ` Alan Mackenzie
  2024-01-31 17:05                     ` Dmitry Gutov
  1 sibling, 1 reply; 18+ messages in thread
From: João Távora @ 2024-01-31 16:46 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Dmitry Gutov, Po Lu, emacs-devel

On Wed, Jan 31, 2024 at 3:32 PM Alan Mackenzie <acm@muc.de> wrote:

> > Was there a way to write it more succinctly, using some higher-level
> > constructs? That is the subject.

Dmitry, sometimes you don't even need higher-level constructs.  You just
need not to freak out over cl-loop, breathe slowly, and expand it
to see what you get.

> I think the discussion is over the advantages and disadvantages of
> replacing obscure concise code with its equivalent in plain Lisp.

Alan, here's a version of log-edit--insert-filled-defuns derived from the one
we had a few days ago, and which really didn't have "countless bugs".
All "plain elisp" as far as I can tell.  Spans one fourth of Po's.  I got
it by simply cleaning up the macroexpansion.

(defun log-edit--insert-filled-defuns (func-names)
  "Insert FUNC-NAMES, following ChangeLog formatting."
  (if (not func-names)
      (insert ":")
    (unless (or (memq (char-before) '(?\n ?\s))
                (> (current-column) fill-column))
      (insert " "))
    (let* ((first-fun t) (def nil))
      (while (consp func-names)
        (setq def (pop func-names))
        (when (>
               (+ (current-column) (string-width def) (if first-fun 1 2)
                  (if (null func-names) 1 0))
               fill-column)
          (unless first-fun (insert ")"))
          (unless (eq (char-before) ?\n) (insert "\n"))
          (setq first-fun t))
        (insert (if first-fun "(" ", ") def)
        (setq first-fun nil)))
    (insert "):")))

Even if in Po's version the comments are not counted, this version is
still less than half the length, less variables, less 'if', no
'delete-char', no 'format', doesn't 'cons', and much more
closely resembles the original, passing all of Po's tests.  Probably
could use a comment to explain the line measuring arithmetic, or
made even simpler if one really wanted to, but I think it's just
short enough to be fine.

Uses 'pop' and 'unless'.  Dunno if that is "plain elisp" to you.
I've never seen that defined anywhere, don't know why you
get to say what it is, or why it's even a useful working concept.

For the other rewritten function it's much of the same
I've already shown how a one-line change would have fixed its bug
(taking for granted that is was even a bug):

-                  (beg (progn (goto-char beg) (line-beginning-position))))
+                  (beg (progn (goto-char beg) (skip-chars-backward "^
\n") (point))))



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: master 78fc49407b8 1/3: Improve filling of ChangeLog entries
  2024-01-31 15:32                   ` Alan Mackenzie
  2024-01-31 16:46                     ` João Távora
@ 2024-01-31 17:05                     ` Dmitry Gutov
  2024-01-31 18:45                       ` Alan Mackenzie
  1 sibling, 1 reply; 18+ messages in thread
From: Dmitry Gutov @ 2024-01-31 17:05 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Po Lu, João Távora, emacs-devel

On 31/01/2024 17:32, Alan Mackenzie wrote:
> Hello, Dmitry.
> 
> On Wed, Jan 31, 2024 at 17:15:41 +0200, Dmitry Gutov wrote:
>> On 31/01/2024 16:01, Po Lu wrote:
>>> See any file in CC Mode,
> 
>> No shortage of maintainers, you say?
> 
> None.

One cannot name a package with bus factor of 1 and say it has plenty of 
people willing to maintain it. I don't mean to criticize your work (not 
knowing the exact tradeoffs), but it's plainly a bad example.

>>> or GCC's reload.cc, which features this massive conditional dwarfing
>>> any of ours:
> 
>> Was there a way to write it more succinctly, using some higher-level
>> constructs? That is the subject.
> 
> I think the discussion is over the advantages and disadvantages of
> replacing obscure concise code with its equivalent in plain Lisp.

Again: Po showed some tapestry of dense code which was supposedly okay. 
What was it supposed to demonstrate? What alternative to compare with?

> After several days of struggling with named-let, cl-labels, and friends,
> I vote for the plain Lisp, even if it does need more lines to express.
> It is simply less work.

I've never used named-let, and very rarely cl-labels.

The latter is a very simple idea, though: create a bunch of local 
function definitions. Like nested functions in Python, for example.



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: master 78fc49407b8 1/3: Improve filling of ChangeLog entries
  2024-01-31 16:46                     ` João Távora
@ 2024-01-31 18:29                       ` Alan Mackenzie
  0 siblings, 0 replies; 18+ messages in thread
From: Alan Mackenzie @ 2024-01-31 18:29 UTC (permalink / raw)
  To: João Távora; +Cc: Dmitry Gutov, Po Lu, emacs-devel

Hello, João.

On Wed, Jan 31, 2024 at 16:46:42 +0000, João Távora wrote:
> On Wed, Jan 31, 2024 at 3:32 PM Alan Mackenzie <acm@muc.de> wrote:

> > > Was there a way to write it more succinctly, using some higher-level
> > > constructs? That is the subject.

> Dmitry, sometimes you don't even need higher-level constructs.  You just
> need not to freak out over cl-loop, breathe slowly, and expand it
> to see what you get.

> > I think the discussion is over the advantages and disadvantages of
> > replacing obscure concise code with its equivalent in plain Lisp.

> Alan, here's a version of log-edit--insert-filled-defuns derived from the one
> we had a few days ago, and which really didn't have "countless bugs".
> All "plain elisp" as far as I can tell.  Spans one fourth of Po's.  I got
> it by simply cleaning up the macroexpansion.

> (defun log-edit--insert-filled-defuns (func-names)
>   "Insert FUNC-NAMES, following ChangeLog formatting."
>   (if (not func-names)
>       (insert ":")
>     (unless (or (memq (char-before) '(?\n ?\s))
>                 (> (current-column) fill-column))
>       (insert " "))
>     (let* ((first-fun t) (def nil))
>       (while (consp func-names)
>         (setq def (pop func-names))
>         (when (>
>                (+ (current-column) (string-width def) (if first-fun 1 2)
>                   (if (null func-names) 1 0))
>                fill-column)
>           (unless first-fun (insert ")"))
>           (unless (eq (char-before) ?\n) (insert "\n"))
>           (setq first-fun t))
>         (insert (if first-fun "(" ", ") def)
>         (setq first-fun nil)))
>     (insert "):")))

Looks OK to me, apart from the doc string which is a bit vague and fails
to define FUNC-NAMES.  But it's better than a lot of existing doc
strings.

> Even if in Po's version the comments are not counted, this version is
> still less than half the length, less variables, less 'if', no
> 'delete-char', no 'format', doesn't 'cons', and much more
> closely resembles the original, passing all of Po's tests.  Probably
> could use a comment to explain the line measuring arithmetic, or
> made even simpler if one really wanted to, but I think it's just
> short enough to be fine.

> Uses 'pop' and 'unless'.  Dunno if that is "plain elisp" to you.

Yes, provided that pop is working on a variable which is a list, not
some fancy "generalised variable".

> I've never seen that defined anywhere, don't know why you
> get to say what it is, or why it's even a useful working concept.

It's not defined, but we all know what it means, just as we know what
"good code" and "bad code" mean.  It's a useful concept, as it can be
contrasted with code full of cl-* functions and macros, pcase, and other
things which make reading and debugging difficult for a lot of people.

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: master 78fc49407b8 1/3: Improve filling of ChangeLog entries
  2024-01-31 17:05                     ` Dmitry Gutov
@ 2024-01-31 18:45                       ` Alan Mackenzie
  2024-01-31 20:39                         ` Dmitry Gutov
  0 siblings, 1 reply; 18+ messages in thread
From: Alan Mackenzie @ 2024-01-31 18:45 UTC (permalink / raw)
  To: Dmitry Gutov; +Cc: Po Lu, João Távora, emacs-devel

Hello, Dmitry.

On Wed, Jan 31, 2024 at 19:05:57 +0200, Dmitry Gutov wrote:
> On 31/01/2024 17:32, Alan Mackenzie wrote:
> > On Wed, Jan 31, 2024 at 17:15:41 +0200, Dmitry Gutov wrote:
> >> On 31/01/2024 16:01, Po Lu wrote:
> >>> See any file in CC Mode,

> >> No shortage of maintainers, you say?

> > None.

> One cannot name a package with bus factor of 1 and say it has plenty of 
> people willing to maintain it.

What's a "bus factor" in this context?  It would appear that one person,
me, is indeed enough to maintain it.  The rate of bugs reported for it
has sunk to near zero, possibly because of the release of the tree sitter
C Mode in Emacs 29.1.

> I don't mean to criticize your work (not knowing the exact tradeoffs),
> but it's plainly a bad example.

Bad example of what?

[ .... ]

> > After several days of struggling with named-let, cl-labels, and friends,
> > I vote for the plain Lisp, even if it does need more lines to express.
> > It is simply less work.

> I've never used named-let, and very rarely cl-labels.

I've never used either, but still need to debug them.  :-(

> The latter is a very simple idea, though: create a bunch of local 
> function definitions. Like nested functions in Python, for example.

It's a complicated macro.  Why is it needed at all?  Is there anything
using it that couldn't be conveniently written in plain Lisp?

-- 
Alan Mackenzie (Nuremberg, Germany).



^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: master 78fc49407b8 1/3: Improve filling of ChangeLog entries
  2024-01-31 18:45                       ` Alan Mackenzie
@ 2024-01-31 20:39                         ` Dmitry Gutov
  0 siblings, 0 replies; 18+ messages in thread
From: Dmitry Gutov @ 2024-01-31 20:39 UTC (permalink / raw)
  To: Alan Mackenzie; +Cc: Po Lu, João Távora, emacs-devel

On 31/01/2024 20:45, Alan Mackenzie wrote:
> Hello, Dmitry.
> 
> On Wed, Jan 31, 2024 at 19:05:57 +0200, Dmitry Gutov wrote:
>> On 31/01/2024 17:32, Alan Mackenzie wrote:
>>> On Wed, Jan 31, 2024 at 17:15:41 +0200, Dmitry Gutov wrote:
>>>> On 31/01/2024 16:01, Po Lu wrote:
>>>>> See any file in CC Mode,
> 
>>>> No shortage of maintainers, you say?
> 
>>> None.
> 
>> One cannot name a package with bus factor of 1 and say it has plenty of
>> people willing to maintain it.
> 
> What's a "bus factor" in this context?

The smallest number of developers who would need to mysteriously 
disappear, for it to become a problem for the project.

> It would appear that one person,
> me, is indeed enough to maintain it.  The rate of bugs reported for it
> has sunk to near zero, possibly because of the release of the tree sitter
> C Mode in Emacs 29.1.

That's good.

>> I don't mean to criticize your work (not knowing the exact tradeoffs),
>> but it's plainly a bad example.
> 
> Bad example of what?

Of a body of Lisp code maintained by different developers, who all 
accepted (and possibly chosen) its current style, thereby justifying it 
as something usable as a standard, rather than only a personal preference.

> [ .... ]
> 
>>> After several days of struggling with named-let, cl-labels, and friends,
>>> I vote for the plain Lisp, even if it does need more lines to express.
>>> It is simply less work.
> 
>> I've never used named-let, and very rarely cl-labels.
> 
> I've never used either, but still need to debug them.  :-(

edebug usually helps, no?

>> The latter is a very simple idea, though: create a bunch of local
>> function definitions. Like nested functions in Python, for example.
> 
> It's a complicated macro.  Why is it needed at all?  Is there anything
> using it that couldn't be conveniently written in plain Lisp?

Having local variable bindings with lambda values is a little messier in 
comparison. For example, the use of cl-labels in 
'comp-collect-rev-post-order' seems easy to read and thus justified. 
Also, it allows having mutually recursive functions, which won't be an 
option in sequential bindings (but I don't see this property taken 
advantage of much).



^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2024-01-31 20:39 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <170643232559.30479.16631610453249222615@vcs2.savannah.gnu.org>
     [not found] ` <20240128085846.187A2C1DAE4@vcs2.savannah.gnu.org>
2024-01-28 13:22   ` master 78fc49407b8 1/3: Improve filling of ChangeLog entries Dmitry Gutov
2024-01-28 13:37     ` Po Lu
2024-01-30 22:07     ` João Távora
2024-01-31  6:43       ` Po Lu
2024-01-31 10:28         ` João Távora
2024-01-31 11:19           ` Po Lu
2024-01-31 13:22             ` João Távora
2024-01-31 14:01               ` Po Lu
2024-01-31 14:30                 ` João Távora
2024-01-31 15:15                 ` Dmitry Gutov
2024-01-31 15:32                   ` Alan Mackenzie
2024-01-31 16:46                     ` João Távora
2024-01-31 18:29                       ` Alan Mackenzie
2024-01-31 17:05                     ` Dmitry Gutov
2024-01-31 18:45                       ` Alan Mackenzie
2024-01-31 20:39                         ` Dmitry Gutov
2024-01-29 20:13   ` Stefan Kangas
2024-01-30  1:26     ` Po Lu

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.