All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.