diff mbox series

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

Message ID 20190129193818.8645-8-jeremyhu@apple.com
State New, archived
Headers show
Series Differences between git-2.20.1 and Apple Git-116 | expand

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
diff mbox series

Patch

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);