git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Adam Tkac <atkac@redhat.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: Jakub Narebski <jnareb@gmail.com>,
	git@vger.kernel.org, Jochen Schmitt <Jochen@herr-schmitt.de>
Subject: Re: [PATCH] Pass "-O xhtml" param to highlight instead of "-xhtml"
Date: Thu, 27 Jan 2011 13:03:23 +0100	[thread overview]
Message-ID: <20110127120323.GA3416@traged.englab.brq.redhat.com> (raw)
In-Reply-To: <7vaainjq7f.fsf@alter.siamese.dyndns.org>

On Wed, Jan 26, 2011 at 01:43:48PM -0800, Junio C Hamano wrote:
> Jakub Narebski <jnareb@gmail.com> writes:
> 
> >> Current highlight utility doesn't recognize "--xhtml" parameter, it
> >> recognizes only "-O xhtml" parameter.
> >> 
> >> Reference: https://bugzilla.redhat.com/show_bug.cgi?id=672293
> >
> > Shouldn't the above be in commit message?
> 
> Actually, I prefer not having that "Reference" in the commit message
> myself, but I do want to have some relevant details
> missing from the proposed commit message but is in
> that ticket (especially the comment #5) in the commit message.
> 
> > In highlight 2.4.5 '-O' means "name of output directory", i.e. --outdir.
> > There is no --out-format either.
> 
> Thanks for digging about 2.4.5; in that case, unlike the redhat ticket
> hinted, this change may not be backward compatible enough, as it seems
> that the compatibility goes only back to 3.0.something.

Right you are, I digged too shallowly, thanks for your inspection.

> 
> >
> > WTF this backward incompatibile change in highlight... the only
> > solution that would make it work both for old and for new versions is
> > to rely on the fact that HTML is default output format, i.e.
> >
> >   	open $fd, quote_command(git_cmd(), "cat-file", "blob", $hash)." | ".
> >   	          quote_command($highlight_bin).
> >  -	          " --xhtml --fragment --syntax $syntax |"
> >  +	          " --fragment --syntax $syntax |"
> >   		or die_error(500, "Couldn't open file or run syntax highlighter");
> 
> Adam, Jochen?  How does the counterproposal look to you?
> 
> Without knowing much about highlight nor using gitweb in general myself,
> the above looks a bit more reasonable fix to the issue, if the default
> format has been and will stay to be HTML.

The proposal looks like the best (and the only one) correct solution.
I will send updated patch.

Regards, Adam

-- 
Adam Tkac, Red Hat, Inc.

  reply	other threads:[~2011-01-27 12:03 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-01-26 17:11 [PATCH] Pass "-O xhtml" param to highlight instead of "-xhtml" Adam Tkac
2011-01-26 21:34 ` Jakub Narebski
2011-01-26 21:43   ` Junio C Hamano
2011-01-27 12:03     ` Adam Tkac [this message]
2011-01-27 12:51       ` [PATCH] Don't pass "--xhtml" to hightlight in gitweb.perl script Adam Tkac

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

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

  git send-email \
    --in-reply-to=20110127120323.GA3416@traged.englab.brq.redhat.com \
    --to=atkac@redhat.com \
    --cc=Jochen@herr-schmitt.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jnareb@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.
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 NNTP newsgroup(s).