git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: Heba Waly <heba.waly@gmail.com>,
	"Heba Waly via GitGitGadget" <gitgitgadget@gmail.com>
Subject: Re: [PATCH 1/1] add: use advise function to display hints
Date: Thu, 02 Jan 2020 14:47:58 -0800	[thread overview]
Message-ID: <xmqqzhf5cw69.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <xmqqpng1eisc.fsf@gitster-ct.c.googlers.com> (Junio C. Hamano's message of "Thu, 02 Jan 2020 11:54:11 -0800")

Junio C Hamano <gitster@pobox.com> writes:

> Use of advise() function is good for giving hints not just due to
> its yellow coloring (which by the way I find not very readable,
> perhaps because I use black ink on white paper).  One good thing in
> using the advise() API is that the messages can also be squelched
> with advice.* configuration variables.

A side note.

Right now, the advise() API is a bit awkweard to use correctly.
When introducing a new advice message, you would

 * come up with advice.frotz configuration variable

 * define and declare advice_frotz global variable that defaults to
   true

 * sprinkle calls like this:

	if (advice_frotz)
		advise(_("helpful message about frotz"));

I am wondering about two things:

 (1) if we can update the API so that the above can be reduced to
     just adding calls like:

	advise_ng("frotz", _("helpful message about frotz"));

 (2) if such a simplified advise_ng API is a good idea to begin
     with.

There are a few advantages the current API has, but it cuts both
ways.

 - Any new advice toggle MUST be registered to the
   advice.c::advice_config[] table.  This table can later be
   extended in the future to allow a list of the toggles to be
   produced at runtime.

   This can be seen as an easy mechanism to force programmers to
   keep the list up to date.  Or it can also be seen as the source
   of extra work.

 - advise() calls can be made without being guarded by any advice.*
   configuration variable.  In the overly simplified advise_ng() API
   shown above, we cannot expresss a pattern like this:

	if (advice_frotz) {
		... make expensive computation to
		... come up with values that need to be shown
		... in the advise() message
		char *result = expensive_computation(...);

		advise(_("message %s about frotz", result));
		free(result);
	}

   without adding another helper function. e.g.

	if (advise_ng_enabled("frotz")) {
		char *result = expensive_computation(...);

		/*
                 * advise_ng("frotz", _("message %s about frotz", result));
                 * is fine as well, but slightly less efficient as
                 * it would involve another call to *_enabled(), so use
		 * the unconditional form of the call
		 */
		advise_ng_raw(_("message %s about frotz", result));

		free(result);
	}


  reply	other threads:[~2020-01-02 22:48 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-01-02  3:04 [PATCH 0/1] [Outreachy] [RFC] add: use advise function to display hints Heba Waly via GitGitGadget
2020-01-02  3:04 ` [PATCH 1/1] " Heba Waly via GitGitGadget
2020-01-02 19:54   ` Junio C Hamano
2020-01-02 22:47     ` Junio C Hamano [this message]
2020-01-07 10:54       ` Heba Waly
2020-01-07 16:35         ` Junio C Hamano
2020-01-07 23:32           ` Heba Waly
2020-01-06 23:13     ` Emily Shaffer
2020-01-06 23:18       ` Junio C Hamano
2020-01-07  4:19     ` Heba Waly
2020-01-06 23:07   ` Emily Shaffer
2020-01-06 23:13     ` Junio C Hamano
2020-01-07 23:12 ` [PATCH v2 0/1] [Outreachy] " Heba Waly via GitGitGadget
2020-01-07 23:12   ` [PATCH v2 1/1] " Heba Waly via GitGitGadget
2020-01-27 23:52     ` Emily Shaffer
2020-01-29  1:09       ` Heba Waly
2020-01-28  0:00     ` Jonathan Tan
2020-01-29  2:04       ` Heba Waly
2020-01-30  1:11   ` [PATCH v3] add: use advice API " Heba Waly via GitGitGadget
2020-01-30 21:59     ` Junio C Hamano
2020-01-31 11:16       ` Heba Waly
2020-02-05 21:18         ` Junio C Hamano
2020-02-05 22:05           ` Heba Waly
2020-02-05 22:18             ` Junio C Hamano
2020-02-05 23:05               ` Heba Waly
2020-02-05 23:18                 ` Junio C Hamano

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: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=xmqqzhf5cw69.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=heba.waly@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://80x24.org/mirrors/git.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).