[(Apple,Git),07/13] HTML documentation is not provided with Apple's git. Make the error message more on point.
diff mbox series

Message ID 20190129193818.8645-8-jeremyhu@apple.com
State New
Headers show
Series
  • Differences between git-2.20.1 and Apple Git-116
Related show

Commit Message

Jeremy Sequoia Jan. 29, 2019, 7:38 p.m. UTC
Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
---
 builtin/help.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Junio C Hamano Jan. 29, 2019, 11:01 p.m. UTC | #1
Jeremy Huddleston Sequoia <jeremyhu@apple.com> writes:

> Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
> ---
>  builtin/help.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/help.c b/builtin/help.c
> index 7739a5c155..e001b6157c 100644
> --- a/builtin/help.c
> +++ b/builtin/help.c
> @@ -383,7 +383,7 @@ static void get_html_page_path(struct strbuf *page_path, const char *page)
>  	if (!strstr(html_path, "://")) {
>  		if (stat(mkpath("%s/git.html", html_path), &st)
>  		    || !S_ISREG(st.st_mode))
> -			die("'%s': not a documentation directory.", html_path);
> +			die("HTML documentation is not provided by this distribution of git.");

Mentioning HTML in the message may be a good idea, but I feel that
"distribution of git" is not something we should say in the source
for those who are building from the source.  Distributors are free
to munge before they generate their binary distribution, of course
;-).

>  	}
>  
>  	strbuf_init(page_path, 0);
Johannes Schindelin Jan. 30, 2019, 1:45 p.m. UTC | #2
Hi Junio,

On Tue, 29 Jan 2019, Junio C Hamano wrote:

> Jeremy Huddleston Sequoia <jeremyhu@apple.com> writes:
> 
> > Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
> > ---
> >  builtin/help.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/builtin/help.c b/builtin/help.c
> > index 7739a5c155..e001b6157c 100644
> > --- a/builtin/help.c
> > +++ b/builtin/help.c
> > @@ -383,7 +383,7 @@ static void get_html_page_path(struct strbuf *page_path, const char *page)
> >  	if (!strstr(html_path, "://")) {
> >  		if (stat(mkpath("%s/git.html", html_path), &st)
> >  		    || !S_ISREG(st.st_mode))
> > -			die("'%s': not a documentation directory.", html_path);
> > +			die("HTML documentation is not provided by this distribution of git.");
> 
> Mentioning HTML in the message may be a good idea, but I feel that
> "distribution of git" is not something we should say in the source
> for those who are building from the source.  Distributors are free
> to munge before they generate their binary distribution, of course
> ;-).

So maybe something like

#ifdef MISSING_HTML_MESSAGE
			die(_(MISSING_HTML_MESSAGE));
#else
			die("'%s': not a documentation directory.", html_path);
#endif

?

Ciao,
Johannes

> 
> >  	}
> >  
> >  	strbuf_init(page_path, 0);
>
Junio C Hamano Jan. 30, 2019, 4:50 p.m. UTC | #3
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Hi Junio,
>
> On Tue, 29 Jan 2019, Junio C Hamano wrote:
>
>> Jeremy Huddleston Sequoia <jeremyhu@apple.com> writes:
>> 
>> > Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
>> > ---
>> >  builtin/help.c | 2 +-
>> >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >
>> > diff --git a/builtin/help.c b/builtin/help.c
>> > index 7739a5c155..e001b6157c 100644
>> > --- a/builtin/help.c
>> > +++ b/builtin/help.c
>> > @@ -383,7 +383,7 @@ static void get_html_page_path(struct strbuf *page_path, const char *page)
>> >  	if (!strstr(html_path, "://")) {
>> >  		if (stat(mkpath("%s/git.html", html_path), &st)
>> >  		    || !S_ISREG(st.st_mode))
>> > -			die("'%s': not a documentation directory.", html_path);
>> > +			die("HTML documentation is not provided by this distribution of git.");
>> 
>> Mentioning HTML in the message may be a good idea, but I feel that
>> "distribution of git" is not something we should say in the source
>> for those who are building from the source.  Distributors are free
>> to munge before they generate their binary distribution, of course
>> ;-).
>
> So maybe something like
>
> #ifdef MISSING_HTML_MESSAGE
> 			die(_(MISSING_HTML_MESSAGE));
> #else
> 			die("'%s': not a documentation directory.", html_path);
> #endif
>
> ?

No, distributors can fork and build from patched source.  What I
meant was along these lines:

    die(_("HTML documentation not installed in '%s'."), html_path));
    die(_("The installer chose to omit HTML docs from '%s''.", html_path));
Johannes Schindelin Jan. 30, 2019, 7:34 p.m. UTC | #4
Hi Junio,

On Wed, 30 Jan 2019, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > On Tue, 29 Jan 2019, Junio C Hamano wrote:
> >
> >> Jeremy Huddleston Sequoia <jeremyhu@apple.com> writes:
> >> 
> >> > Signed-off-by: Jeremy Huddleston Sequoia <jeremyhu@apple.com>
> >> > ---
> >> >  builtin/help.c | 2 +-
> >> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >> >
> >> > diff --git a/builtin/help.c b/builtin/help.c
> >> > index 7739a5c155..e001b6157c 100644
> >> > --- a/builtin/help.c
> >> > +++ b/builtin/help.c
> >> > @@ -383,7 +383,7 @@ static void get_html_page_path(struct strbuf *page_path, const char *page)
> >> >  	if (!strstr(html_path, "://")) {
> >> >  		if (stat(mkpath("%s/git.html", html_path), &st)
> >> >  		    || !S_ISREG(st.st_mode))
> >> > -			die("'%s': not a documentation directory.", html_path);
> >> > +			die("HTML documentation is not provided by this distribution of git.");
> >> 
> >> Mentioning HTML in the message may be a good idea, but I feel that
> >> "distribution of git" is not something we should say in the source
> >> for those who are building from the source.  Distributors are free
> >> to munge before they generate their binary distribution, of course
> >> ;-).
> >
> > So maybe something like
> >
> > #ifdef MISSING_HTML_MESSAGE
> > 			die(_(MISSING_HTML_MESSAGE));
> > #else
> > 			die("'%s': not a documentation directory.", html_path);
> > #endif
> >
> > ?
> 
> No, distributors can fork and build from patched source.  What I
> meant was along these lines:
> 
>     die(_("HTML documentation not installed in '%s'."), html_path));
>     die(_("The installer chose to omit HTML docs from '%s''.", html_path));

Thanks for the clarification. I think your rationale makes a total lot of
sense.

Thank you,
Dscho

Patch
diff mbox series

diff --git a/builtin/help.c b/builtin/help.c
index 7739a5c155..e001b6157c 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -383,7 +383,7 @@  static void get_html_page_path(struct strbuf *page_path, const char *page)
 	if (!strstr(html_path, "://")) {
 		if (stat(mkpath("%s/git.html", html_path), &st)
 		    || !S_ISREG(st.st_mode))
-			die("'%s': not a documentation directory.", html_path);
+			die("HTML documentation is not provided by this distribution of git.");
 	}
 
 	strbuf_init(page_path, 0);