Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing

2013-06-12 Thread Matthieu Moy
Jeff King p...@peff.net writes:

 I was thinking that you would be self-contained inside the
 contrib/mw-to-git directory, and therefore you would have to teach your
 code how to install the Git module, and you could not longer just cp
 git-remote-mediawiki into the right place to install it.

 But I think we have already crossed that bridge somewhat with Git.pm.
 And if you add your module as perl/Git/MediaWiki.pm and use the existing
 perl build system, then it is not any extra effort from the build
 system.

I'm not sure having perl/Git/MediaWiki.pm would be a good idea: this
MediaWiki.pm would be really a mediawiki thing more than a Git thing, so
the Git main tree probably want to stay away from it and keep it in
contrib.

But you should be able to use contrib/mw-to-git/perl/GitMediawiki.pm or
something like that and chain to ../../perl/Makefile in
contrib/mw-to-git/Makefile.

Also, for now, git-remote-mediawiki works only after you run make
install in Git's toplevel. I think that's ok, but it would be weird to
be able to use/test git-remote-mediawiki only after doing a make
install to deploy the new mediawiki Perl module.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing

2013-06-12 Thread Jeff King
On Wed, Jun 12, 2013 at 08:55:12AM +0200, Matthieu Moy wrote:

  But I think we have already crossed that bridge somewhat with Git.pm.
  And if you add your module as perl/Git/MediaWiki.pm and use the existing
  perl build system, then it is not any extra effort from the build
  system.
 
 I'm not sure having perl/Git/MediaWiki.pm would be a good idea: this
 MediaWiki.pm would be really a mediawiki thing more than a Git thing, so
 the Git main tree probably want to stay away from it and keep it in
 contrib.

Yes, it's ugly. It means that the mediawiki stuff creeps out of contrib
and into the main tree; and worse, as part of a public API that gets
installed. We'd have to be a lot more picky about the interface and the
code quality.

 But you should be able to use contrib/mw-to-git/perl/GitMediawiki.pm or
 something like that and chain to ../../perl/Makefile in
 contrib/mw-to-git/Makefile.

That might work. Most of the magic in perl/Makefile happens in the
perl-generated MakeMaker bits, though, so it may not be that easy. I
haven't looked.

 Also, for now, git-remote-mediawiki works only after you run make
 install in Git's toplevel. I think that's ok, but it would be weird to
 be able to use/test git-remote-mediawiki only after doing a make
 install to deploy the new mediawiki Perl module.

Good point.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing

2013-06-11 Thread Benoît Person
The V2 is on the launchpad but I am still struggling with the code
factoring between git-mw.perl and git-remote-mediawiki.perl :/ .

On 9 June 2013 08:08, Jeff King p...@peff.net wrote:

 You could make a Git::MediaWiki.pm module, but installing that would
 significantly complicate the build procedure, and potentially be
 annoying for users. One trick I have done in the past is to concatenate
 bits of perl script together in the Makefile, like this:

   foo: common.pl foo.pl
   { \
 echo '$(PERL_PATH_SQ)'  \
 for i in $^; do \
   echo #line 1 $src  \
 cat $src \
 done \
   } $@+
   mv $@+ $@

 That would conflict a bit with the way we chain to git's Makefile,
 though. I suspect you could do something complicated like build foo.pl
 from common.pl and foo-main.pl, then chain to git's Makefile to
 build foo from foo.pl.

I've implemented this one for now but after a real-life meeting with
Matthieu Moy we discussed the possibility to build a GitMediawiki.pm
module. It seems more clean than the concatenation of perl scripts.
Plus, it would force people to limit side effects inside the functions
used in this package/utils file (I have in mind the mw_connect_maybe
function here and a couple of others which directly *hope* for global
vars to be set to a nice value before being called).

What I find bad in the concatenating-thingy is the mandatory rename of
git-mw.perl into something like git-mw.unmerged.perl and
git-remote-mediawiki.perl into git-remote-mediawiki.unmerged.perl.
Otherwise, like you said, it would be hard to chain to git's Makefile
after the merge.

For now, I have really no idea which one is the best. If I may ask,
what did you have in mind while saying:

 You could make a Git::MediaWiki.pm module, but installing that would
 significantly complicate the build procedure, and potentially be
 annoying for users.

?

Since my previous commit (ea07ec1 in next - use Git.pm functions for
credentials), git-remote-mediawiki.perl already depends on the proper
installation of the Git.pm package. In what ways the need for the
installation of yet another package (GitMediawiki.pm) would annoy a
user ?

Thank you for your time,

Benoit
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing

2013-06-11 Thread Jeff King
On Tue, Jun 11, 2013 at 11:31:31PM +0200, Benoît Person wrote:

 I've implemented this one for now but after a real-life meeting with
 Matthieu Moy we discussed the possibility to build a GitMediawiki.pm
 module. It seems more clean than the concatenation of perl scripts.
 Plus, it would force people to limit side effects inside the functions
 used in this package/utils file (I have in mind the mw_connect_maybe
 function here and a couple of others which directly *hope* for global
 vars to be set to a nice value before being called).
 
 What I find bad in the concatenating-thingy is the mandatory rename of
 git-mw.perl into something like git-mw.unmerged.perl and
 git-remote-mediawiki.perl into git-remote-mediawiki.unmerged.perl.
 Otherwise, like you said, it would be hard to chain to git's Makefile
 after the merge.

Yeah, I agree it's very hacky.

 For now, I have really no idea which one is the best. If I may ask,
 what did you have in mind while saying:
 
  You could make a Git::MediaWiki.pm module, but installing that would
  significantly complicate the build procedure, and potentially be
  annoying for users.
 
 Since my previous commit (ea07ec1 in next - use Git.pm functions for
 credentials), git-remote-mediawiki.perl already depends on the proper
 installation of the Git.pm package. In what ways the need for the
 installation of yet another package (GitMediawiki.pm) would annoy a
 user ?

I was thinking that you would be self-contained inside the
contrib/mw-to-git directory, and therefore you would have to teach your
code how to install the Git module, and you could not longer just cp
git-remote-mediawiki into the right place to install it.

But I think we have already crossed that bridge somewhat with Git.pm.
And if you add your module as perl/Git/MediaWiki.pm and use the existing
perl build system, then it is not any extra effort from the build
system.

That is probably the most sane way to go.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing

2013-06-09 Thread Jeff King
On Fri, Jun 07, 2013 at 11:50:31PM +0200, benoit.per...@ensimag.fr wrote:

 The #7 issue on git-mediawiki's issue tracker [1] states that the ability to
 preview content without pushing would be a nice thing to have.

Sounds like a useful goal.

 The default behaviour for the `preview` subcommand is:
 1- Find the remote name of the current branch's upstream and check if it's a
 wiki one with its url (ie: mediawiki://)
 2- Parse the content of the local file (given as argument) using the distant
 wiki's API.

Makes sense.

 3- Retrieve the current page on the distant mediawiki.
 4- Merge those those contents.

I'm not sure what these steps are for. You are trying to preview not
just your local version, but pulling in any changes that have happened
upstream since the work you built on top of?

It seems like that is a separate, orthogonal issue, and git would be the
right place to do that merge. IOW, as a user, your workflow would be
something like:

  1. git fetch, pulling down the latest copy from the server

  2. make your changes on top

  3. use this command to preview your changes

  4. use git fetch to check whether anything else happened on the
 server.

   a. If not, go ahead and push.

   b. If upstream changed, use git diff to inspect the changes to
  the wiki source. Merge or rebase your changes with respect to
  what's on the server. Goto step 3 to make sure they look good.

I also wonder if it would be useful to be able to specify not only files
in the filesystem, but also arbitrary blobs. So in 4b above, you could
git mw preview origin:page.mw to see the rendered version of what
upstream has done.

 It works but a couple of points trouble me: 
 1-  I had to copy two functions from `git-remote-mediawiki.perl`, I don't 
 really know how we could factorize those things ? I don't think it 
 makes 
 much sense to create a package just for them ?

You could make a Git::MediaWiki.pm module, but installing that would
significantly complicate the build procedure, and potentially be
annoying for users. One trick I have done in the past is to concatenate
bits of perl script together in the Makefile, like this:

  foo: common.pl foo.pl
  { \
echo '$(PERL_PATH_SQ)'  \
for i in $^; do \
  echo #line 1 $src  \
cat $src \
done \
  } $@+
  mv $@+ $@

That would conflict a bit with the way we chain to git's Makefile,
though. I suspect you could do something complicated like build foo.pl
from common.pl and foo-main.pl, then chain to git's Makefile to
build foo from foo.pl.

 2-  The current behavior is to crash if the current branch do not have an
 upstream branch on a valid mediawiki remote. To find that specific 
 remote, 
 it runs `git rev-parse --symbolic-full-name @{upstream}` which will 
 return 
 something like `/refs/remotes/$remote_name/master`.
   2a-  maybe there is a better way to find that remote name ?

If you just care about the remote name and not the name of the local
branch, you can just ask for

  my $curr_branch = `git symbolic-ref HEAD`;
  my $remote = `git config branch.$curr_branch.remote`;
  my $url = `git config remote.$remote.url`;

Of course you would want some error checks and probably some chomp()s in
there, too.

   2b-  would it be useful to add a fallback if that search fails ? searching 
for a valid mediawiki remote url in all the remotes returned by 
`git remote` for instance ?

That is probably OK as long as there is only one such remote, and it
would help the case where you have branched off of a local branch (so
your upstream remote is .).  If there are two mediawiki remotes,
though, it would make sense to simply fail, as you don't know which to
use. But I'd expect the common case by far to be that you simply have
one such remote.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing

2013-06-09 Thread Jeff King
On Sat, Jun 08, 2013 at 09:00:30PM +0200, Matthieu Moy wrote:

  +   # Auto-loading in browser
  +   if ($autoload) {
  +   open(my $browser, -|:encoding(UTF-8), xdg-open 
  .$preview_file_name);
 
 That could be read from Git's configuration, and default to xdg-open.
 But you don't want to hardcode it in the middle of the code.

In fact, I think we provide the git-web--browse tool for just this
purpose. It takes care of the trickiness of looking at the web.browser
config, resolving custom browser tools defined by browser.tool.*,
and handling backgrounding of the browser (which you want to do for
graphical browsers but not for terminal ones).

-Peff

PS I agreed with all of the other comments in your review. :)
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing

2013-06-09 Thread Matthieu Moy
Jeff King p...@peff.net writes:

 1- Find the remote name of the current branch's upstream and check if it's a
 wiki one with its url (ie: mediawiki://)
 2- Parse the content of the local file (given as argument) using the distant
 wiki's API.

 Makes sense.

 3- Retrieve the current page on the distant mediawiki.
 4- Merge those those contents.

 I'm not sure what these steps are for. You are trying to preview not
 just your local version, but pulling in any changes that have happened
 upstream since the work you built on top of?

Same question here. I'd expect git mw preview in a mediawiki workflow
to do what pdflatex foo  evince foo.pdf do in a latex workflow: see
in rendered form what I've been doing.

In a latex flow, if I want to see how my local changes merge with the
remote ones, I do git merge  pdflatex, and I'd do the same with git
mw.

 I also wonder if it would be useful to be able to specify not only files
 in the filesystem, but also arbitrary blobs. So in 4b above, you could
 git mw preview origin:page.mw to see the rendered version of what
 upstream has done.

Next step could even be git mw diff $from $to, using the wiki to
render the diff. Not a priority, but could be funny.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing

2013-06-09 Thread Benoît Person
Matthieu Moy matthieu@grenoble-inp.fr writes:
 Same question here. I'd expect git mw preview in a mediawiki workflow
 to do what pdflatex foo  evince foo.pdf do in a latex workflow: see
 in rendered form what I've been doing.

 In a latex flow, if I want to see how my local changes merge with the
 remote ones, I do git merge  pdflatex, and I'd do the same with git
 mw.

In fact, I should not have used merge to describe how the two contents
(page template + new parsed content) are combined together. For
now, the code simply replaces the template page's text content (the one
retrieved from the remote) with the new one. It does not really care if
the remote has changes or not. (And, to be honest, I did not thought
about that issue ;) ).

But, like both of you said : in a typical workflow, the merging would be
left to the user so the current behavior is fine I think ?

 I also wonder if it would be useful to be able to specify not only files
 in the filesystem, but also arbitrary blobs. So in 4b above, you could
 git mw preview origin:page.mw to see the rendered version of what
 upstream has done.
 Next step could even be git mw diff $from $to, using the wiki to
 render the diff. Not a priority, but could be funny.

I searched in the Mediawiki API if there was a way to diff from a stored
revision and raw text content but I've found nothing :/ . We could make
a little hack to do that by saving as a new revision the local content,
and use the DeleteRevision-thingy from Mediawiki [1] to hide this
useless revision but it would floods the remote DB and usually users
to not have the permission to use that tool. So, for now I would say
it's a no-go :/ .

[1] http://www.mediawiki.org/wiki/Manual:RevisionDelete

Benoit Person

 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing

2013-06-09 Thread Benoît Person
On 9 June 2013 08:08, Jeff King p...@peff.net wrote:
 I also wonder if it would be useful to be able to specify not only files
 in the filesystem, but also arbitrary blobs. So in 4b above, you could
 git mw preview origin:page.mw to see the rendered version of what
 upstream has done.

Hum, so `git mw preview origin:page.mw` would just do the get
request to the remote mediawiki, save it locally and - maybe - load it
in the browser ? Is it really better than just opening the browser and
typing the right URL ?

Currently, this URL is one click away when you have preview file loaded
in a web browser.

 It works but a couple of points trouble me:
 1-  I had to copy two functions from `git-remote-mediawiki.perl`, I don't
 really know how we could factorize those things ? I don't think it 
 makes
 much sense to create a package just for them ?

 You could make a Git::MediaWiki.pm module, but installing that would
 significantly complicate the build procedure, and potentially be
 annoying for users. One trick I have done in the past is to concatenate
 bits of perl script together in the Makefile, like this:

   foo: common.pl foo.pl
   { \
 echo '$(PERL_PATH_SQ)'  \
 for i in $^; do \
   echo #line 1 $src  \
 cat $src \
 done \
   } $@+
   mv $@+ $@

 That would conflict a bit with the way we chain to git's Makefile,
 though. I suspect you could do something complicated like build foo.pl
 from common.pl and foo-main.pl, then chain to git's Makefile to
 build foo from foo.pl.

ok, thanks a lot.

 2-  The current behavior is to crash if the current branch do not have an
 upstream branch on a valid mediawiki remote. To find that specific 
 remote,
 it runs `git rev-parse --symbolic-full-name @{upstream}` which will 
 return
 something like `/refs/remotes/$remote_name/master`.
   2a-  maybe there is a better way to find that remote name ?

 If you just care about the remote name and not the name of the local
 branch, you can just ask for

   my $curr_branch = `git symbolic-ref HEAD`;
   my $remote = `git config branch.$curr_branch.remote`;
   my $url = `git config remote.$remote.url`;

 Of course you would want some error checks and probably some chomp()s in
 there, too.

The fact is, `git symbolic-ref HEAD` result would have to be parsed in order
to extract the current branch name like I currently extract the remote name.
So, is it really better than `git rev-parse --symbolic-full-name @{upstream}` ?

   2b-  would it be useful to add a fallback if that search fails ? searching
for a valid mediawiki remote url in all the remotes returned by
`git remote` for instance ?

 That is probably OK as long as there is only one such remote, and it
 would help the case where you have branched off of a local branch (so
 your upstream remote is .).  If there are two mediawiki remotes,
 though, it would make sense to simply fail, as you don't know which to
 use. But I'd expect the common case by far to be that you simply have
 one such remote.

Well, I thought that `git mw preview` could provide an interactive mode
where, if the first search fails, it would find all the mediawiki remotes, and
offers to the user a way to choose the remote ?

Benoit Person

 --
 To unsubscribe from this list: send the line unsubscribe git in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing

2013-06-09 Thread Matthieu Moy
Benoît Person benoit.per...@ensimag.fr writes:

 On 9 June 2013 08:08, Jeff King p...@peff.net wrote:
 I also wonder if it would be useful to be able to specify not only files
 in the filesystem, but also arbitrary blobs. So in 4b above, you could
 git mw preview origin:page.mw to see the rendered version of what
 upstream has done.

 Hum, so `git mw preview origin:page.mw` would just do the get
 request to the remote mediawiki, save it locally and - maybe - load it
 in the browser ? Is it really better than just opening the browser and
 typing the right URL ?

 Currently, this URL is one click away when you have preview file loaded
 in a web browser.

origin:page.mw is not necessarily the best example, but HEAD^:page.mw
can exist in your local history and not on the wiki. Even HEAD:page.mw
can be interesting if you have uncommited changes. Not that it's
terribly useful to be able to preview it, but once you can deal with
local files, it should be easy enough to generalize it to any blob.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing

2013-06-09 Thread Jeff King
On Sun, Jun 09, 2013 at 01:01:48PM +0200, Matthieu Moy wrote:

  I also wonder if it would be useful to be able to specify not only files
  in the filesystem, but also arbitrary blobs. So in 4b above, you could
  git mw preview origin:page.mw to see the rendered version of what
  upstream has done.
 
 Next step could even be git mw diff $from $to, using the wiki to
 render the diff. Not a priority, but could be funny.

I was actually thinking along the same lines. I often do something
similar with Git's documentation. When you are tweaking the formatting
or an asciidoc knob, it is useful to diff the rendered output to check
that the changes had the effect you wanted.

I usually have done so by hand, but I actually wonder if git difftool
could be used as a wrapper for this (I suspect not, because you have to
deal with the whole build tree, not just individual blobs).

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing

2013-06-09 Thread Jeff King
On Sun, Jun 09, 2013 at 02:35:45PM +0200, Benoît Person wrote:

 On 9 June 2013 08:08, Jeff King p...@peff.net wrote:
  I also wonder if it would be useful to be able to specify not only files
  in the filesystem, but also arbitrary blobs. So in 4b above, you could
  git mw preview origin:page.mw to see the rendered version of what
  upstream has done.
 
 Hum, so `git mw preview origin:page.mw` would just do the get
 request to the remote mediawiki, save it locally and - maybe - load it
 in the browser ? Is it really better than just opening the browser and
 typing the right URL ?

Not really, but when you start doing origin^:page.mw, it may make more
of a difference.

  If you just care about the remote name and not the name of the local
  branch, you can just ask for
 
my $curr_branch = `git symbolic-ref HEAD`;
my $remote = `git config branch.$curr_branch.remote`;
my $url = `git config remote.$remote.url`;
 
  Of course you would want some error checks and probably some chomp()s in
  there, too.
 
 The fact is, `git symbolic-ref HEAD` result would have to be parsed in order
 to extract the current branch name like I currently extract the remote name.
 So, is it really better than `git rev-parse --symbolic-full-name @{upstream}` 
 ?

It is, because it is not strictly true that an upstream of
refs/remotes/foo/* is for the remote foo (though in 99% of cases, it
is). To find the upstream, git looks at branch.$curr_branch.remote, then
calculates the upstream based on the fetch refspecs. Then you try to
undo that by converting it back from the right-hand side of the fetch
refspec into a remote name. It would be much easier to just look at the
config. :)

And yes, you do need the short name of the branch from HEAD, not the
full refname. You can use git symbolic-ref --short for that. You also
should check that it returns something useful (i.e., that we are not on
a detached HEAD).

  That is probably OK as long as there is only one such remote, and it
  would help the case where you have branched off of a local branch (so
  your upstream remote is .).  If there are two mediawiki remotes,
  though, it would make sense to simply fail, as you don't know which to
  use. But I'd expect the common case by far to be that you simply have
  one such remote.
 
 Well, I thought that `git mw preview` could provide an interactive mode
 where, if the first search fails, it would find all the mediawiki remotes, and
 offers to the user a way to choose the remote ?

That's fine; just as long as we do not silently produce output from an
unknown source when there is ambiguity.

You can do an interactive selection, or even just say something like:

  There are multiple mediawiki remotes, and I do not know which one you
  want to use for your preview. Which of:

remote1
remote2

  did you mean? Try using the -r option to specify the remote.

and then let the user repeat their command with the -r option using
shell history. That saves you from having to write an interactive
component, and teaches the user how to script it.

-Peff
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing

2013-06-08 Thread Matthieu Moy
benoit.per...@ensimag.fr writes:

 From: Benoit Person benoit.per...@ensimag.fr

 The #7 issue on git-mediawiki's issue tracker [1] states that the ability to
 preview content without pushing would be a nice thing to have.

 This commit is a first attempt to achieve it. It adds a new git command,
 named `git mw`. This command accepts the subcommands `help` and `preview`
 for now.

Review could be easier if you have PATCH 1/2 introducing the command
with only help (essentially to check that the build system works), and
then focus on preview.

I won't insist in splitting this particular commit, just take it as an
advice for future work.

 --- a/contrib/mw-to-git/Makefile
 +++ b/contrib/mw-to-git/Makefile
 @@ -5,13 +5,17 @@
  ## Build git-remote-mediawiki
  
  SCRIPT_PERL=git-remote-mediawiki.perl
 +SCRIPT_PERL_MW=git-mw.perl

Why do you need another variable? Just adding git-mw.perl to SCRIPT_PERL
should do it, no? (Well, probably the make SCRIPT_PERL=$(SCRIPT_PERL)
lacks quotes around the argument, but then you should fix that).

 +# Constants
 +# Mediawiki filenames can contain forward slashes. This variable decides by 
 which pattern they should be replaced
 +Readonly::scalar $SLASH_REPLACEMENT = %2F;

This one is copied from git-remote-mediawiki.perl, and is a
git-mediawiki specific thing, so it would not make sense to put it into
Git.pm. This starts convincing me that we should have a GitMediawiki.pm
or so, and both scripts should use it.

Another option would be to put everything in git-remote-mediawiki.perl,
and probably to have git mw as a wrapper around it (to avoid having to
type git remote-mediawiki which is a bit long).

But the script starts being a bit long, so I think it makes more sense
to split it. So I'd say

PATCH 1/3 : introduce git mw
PATCH 2/3 : move sharable code to a new module (and make sure it's
installed properly by make install)
PATCH 3/3 : actually implement the preview feature

Perhaps others will have other/better advices.

 + # Auto-loading in browser
 + if ($autoload) {
 + open(my $browser, -|:encoding(UTF-8), xdg-open 
 .$preview_file_name);

That could be read from Git's configuration, and default to xdg-open.
But you don't want to hardcode it in the middle of the code.

Also, why use open if you don't want to communicate with the process?
system would be sufficient, and won't need close.

Prefer passing an array of arguments, instead of concatenating strings
and then rely on command-line parsing to re-split it.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH/RFC] git-remote-mediawiki: new tool to preview local changes without pushing

2013-06-07 Thread benoit . person
From: Benoit Person benoit.per...@ensimag.fr

The #7 issue on git-mediawiki's issue tracker [1] states that the ability to
preview content without pushing would be a nice thing to have.

This commit is a first attempt to achieve it. It adds a new git command,
named `git mw`. This command accepts the subcommands `help` and `preview`
for now.

The default behaviour for the `preview` subcommand is:
1- Find the remote name of the current branch's upstream and check if it's a
wiki one with its url (ie: mediawiki://)
2- Parse the content of the local file (given as argument) using the distant
wiki's API.
3- Retrieve the current page on the distant mediawiki.
4- Merge those those contents.
5- Convert relative links to absolute ones.
6- Save the result on disk.

The command also accepts argument for more controls:
  --autoload | -a tries to launch the newly generated file in the user's 
  default browser
  --remote | -r provides a way to select the distant mediawiki in which the 
user wants to preview his file.
  --output | -o enables the user to choose the output filename. Default output 
filename is based on the input filename in which the extension
`.mw` is replaced with `.html`.

It works but a couple of points trouble me: 
1-  I had to copy two functions from `git-remote-mediawiki.perl`, I don't 
really know how we could factorize those things ? I don't think it makes 
much sense to create a package just for them ?
2-  The current behavior is to crash if the current branch do not have an
upstream branch on a valid mediawiki remote. To find that specific remote, 
it runs `git rev-parse --symbolic-full-name @{upstream}` which will return 
something like `/refs/remotes/$remote_name/master`.
  2a-  maybe there is a better way to find that remote name ?
  2b-  would it be useful to add a fallback if that search fails ? searching 
   for a valid mediawiki remote url in all the remotes returned by 
   `git remote` for instance ?
3-  The current idea of merging the content of the mediawiki's current page 
with new content fails when the page is a local creation. (Hence the 
error message when we get a bad result from the `get` call l.129). I 
thought about two ways to overcome this:
  3a-  Use the wiki's homepage for the template.
  3b-  A two-step process:
   - first we create a general template (based on the wiki's homepage) 
 with a specific flag at the content position. This step is done only 
 if that template do not exists locally.
   - replace the specific flag with the newly parsed content.
   The downfall of those two solutions is on links like 'talk', 
   'view source' etc ... those will need to be updated to. And evven then, 
   it will still fails for page created only locally.
4-  In the Makefile, there is certainly a more Makefily way to do it but I 
had no luck finding it and still preserving the factorization for the 
`build`, `install` and `clean` target. I ended up with something like this:

build: $(BUILD_SCRIPTS)
$(BUILD_SCRIPTS):
$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$@ \
build-perl-script

install: ...
clean: ...

but clearly, for only two scripts, it's more a refucktoring than a 
refactoring :/ .

[1] https://github.com/moy/Git-Mediawiki/issues/7


Signed-off-by: Benoit Person benoit.per...@ensimag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr

---
 contrib/mw-to-git/Makefile|   4 +
 contrib/mw-to-git/git-mw.perl | 249 ++
 2 files changed, 253 insertions(+)
 create mode 100644 contrib/mw-to-git/git-mw.perl

diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile
index f149719..0cba1e3 100644
--- a/contrib/mw-to-git/Makefile
+++ b/contrib/mw-to-git/Makefile
@@ -5,13 +5,17 @@
 ## Build git-remote-mediawiki
 
 SCRIPT_PERL=git-remote-mediawiki.perl
+SCRIPT_PERL_MW=git-mw.perl
 GIT_ROOT_DIR=../..
 HERE=contrib/mw-to-git/
 
 SCRIPT_PERL_FULL=$(patsubst %,$(HERE)/%,$(SCRIPT_PERL))
+SCRIPT_PERL_MW_FULL=$(patsubst %,$(HERE)/%,$(SCRIPT_PERL_MW))
 
 all: build
 
 build install clean:
$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
 $@-perl-script
+   $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_MW_FULL) \
+$@-perl-script
\ No newline at end of file
diff --git a/contrib/mw-to-git/git-mw.perl b/contrib/mw-to-git/git-mw.perl
new file mode 100644
index 000..a92c459
--- /dev/null
+++ b/contrib/mw-to-git/git-mw.perl
@@ -0,0 +1,249 @@
+#! /usr/bin/perl
+
+# Copyright (C) 2013
+# Benoit Person benoit.per...@ensimag.imag.fr
+# Celestin Matte celestin.ma...@ensimag.imag.fr
+# License: GPL v2 or later
+
+# Tools attached to git-remote-mediawiki (help, preview ...)
+# Documentation  bugtracker: https://github.com/moy/Git-Mediawiki/
+
+use strict;
+use warnings;
+
+use Git;
+use