emacs-orgmode@gnu.org archives
 help / color / mirror / code / Atom feed
From: Matt Huszagh <huszaghmatt@gmail.com>
To: Ihor Radchenko <yantar92@gmail.com>
Cc: Emacs-Orgmode <emacs-orgmode@gnu.org>
Subject: Re: [PATCH] Remove additional newline at end of results block
Date: Fri, 08 Jul 2022 16:50:35 -0700	[thread overview]
Message-ID: <8735fb6u5w.fsf@gmail.com> (raw)
In-Reply-To: <87zghsi3ki.fsf@localhost>

Ihor Radchenko <yantar92@gmail.com> writes:

> Thanks for the patch! And sorry for the late reply.

No worries for the delay - I know you have a lot of these. Thanks for
the review.

> Your explanation is quite confusing, but I think I managed to understand
> what you referred to:
>
> ------
>  We can write the simplest equation as
>  #+begin_src latex :exports results
>  \begin{equation}
>    1 + 1 = 2,
>  \end{equation}
>  #+end_src
> the paragraph should continue here.
> --------
>
> will be exported as
>
> ------
> We can write the simplest equation as
> \begin{equation}
>   1 + 1 = 2,
> \end{equation}
>
> the paragraph should continue here.
> ------
>
> Note the extra empty line below equation environment.
>
> However, I am not confident if the proposed change is going to be safe
> for other uses of src blocks.
>
> I'd like to request other people who use export and source blocks
> extensively to try the patch and see if it breaks anything.
>
> Meanwhile, could you please reword the commit message and make it more
> concise and clear?

Can you clarify what you find to be unclear? Rereading my own commit
message, my only problem with it is how it starts: I'd add one sentence
to contextualize it a bit. For instance,

The previous behavior always ensured the presence of an empty line
between the results block of a source block and the subsequent
text. However, inserting this newline prevents a valid use-case and
protects against an edge-case that is completely avoidable without the
additional guarantee it provides. ... (the rest remains unchanged)

Oh and I also made a typo. A sentence further down should read
"...generate an SVG image...".

This commit message isn't short, but I think it's very clear. It
describes the previous behavior, explains the rationale for that
behavior, and then illustrates (with a complete example) how this
prevents a valid use case. It also explains why the new change does not
prohibit any behavior that was previously possible.

As someone who frequently uses git log, I'd much rather see a commit
message like this than the typical (usually) unhelpful one or two
sentences that fail to provide the motivation for a change. There's no
downside to a long commit message, and this one is structured such that
it proceeds from more general to more specific information - not
everyone has to read the entire thing.

I'm not trying to be difficult :) but I do care about the quality of
this codebase (as I know do you), so I'm reluctant to change something
in a way I feel is inferior. But, if you have specific parts etc you
feel are unclear I'm more than happy to address those.

Matt


  reply	other threads:[~2022-07-08 23:52 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-12-30 15:25 [PATCH] Remove additional newline at end of results block Matt Huszagh
2022-07-02  3:41 ` Ihor Radchenko
2022-07-08 23:50   ` Matt Huszagh [this message]
2022-07-09  7:28   ` Ihor Radchenko

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.orgmode.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8735fb6u5w.fsf@gmail.com \
    --to=huszaghmatt@gmail.com \
    --cc=emacs-orgmode@gnu.org \
    --cc=yantar92@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).