Re: [PATCH v8 3/7] git-remote-mediawiki: New git bin-wrapper for developement

2013-07-06 Thread Benoît Person
On 5 July 2013 09:04, Matthieu Moy matthieu@grenoble-inp.fr wrote:
 benoit.per...@ensimag.fr writes:

 --- a/contrib/mw-to-git/Makefile
 +++ b/contrib/mw-to-git/Makefile
 @@ -2,6 +2,12 @@
  # Copyright (C) 2013
  # Matthieu Moy matthieu@imag.fr
  #
 +# To build and test:
 +#
 +#   make:
 +# bin-wrapper/git mw preview Some_page.mw
 +# bin-wrapper/git clone mediawiki::http://example.com/wiki/
 +#

 The colon after make and the indentation look weird. Shouldn't this be

 +# To build and test:
 +#
 +#   make
 +#   bin-wrapper/git mw preview Some_page.mw
 +#   bin-wrapper/git clone mediawiki::http://example.com/wiki/
 +#

 ?

oops, yes definitely.
--
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 v7 2/7] wrap-for-bin: Make bin-wrappers chainable

2013-07-03 Thread Benoît Person
 For now, GITPERLLIB is only used in that kind of statements:
 use lib (split(/:/, $ENV{GITPERLLIB} || ... ));

 The trailing ':' does not really matter, split will ignore it.

 That may be true with the current use, but For now leaves funny
 taste, doesn't it?
definitely. For me, the issue was that we were trading that funny
taste for less
readable code in wrap-for-bin.sh but with that default-substitution
construct, it seems
worth it. (a huge if-block seemed even funnier in fact :) )

 It is not too much trouble to fix by writing

   GITPERLLIB='@@BUILD_DIR@@/perl/blib/lib'${GITPERLLIB:+:$GITPERLLIB}

 and we will not have to be worried about future changes breaking
 today's assumption.
That construct is really nice :) did not know there was such a thing
in bash, thanks.
--
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 v7 2/7] wrap-for-bin: Make bin-wrappers chainable

2013-07-02 Thread Benoît Person
 Do we want to add that ':' unconditionally?  Could GITPERLLIB be
 empty?

For now, GITPERLLIB is only used in that kind of statements:
use lib (split(/:/, $ENV{GITPERLLIB} || ... ));

The trailing ':' does not really matter, split will ignore it.

With the current codebase, I think it's nicer to do it that way (makes
wrap-for-bin.sh more readable).
--
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 v6 2/5] git-remote-mediawiki: new git bin-wrapper for developement

2013-06-29 Thread Benoît Person
 GIT_ROOT_DIR=../../..
 GIT_EXEC_PATH=$(cd $(dirname $0)  cd ${GIT_ROOT_DIR}  pwd)
 GIT_MEDIAWIKI=$GIT_EXEC_PATH/contrib/mw-to-git
 PATH=$GIT_MEDIAWIKI/contrib/mw-to-git:$PATH
 GPLEXTRA=$GIT_MEDIAWIKI/contrib/mw-to-git

 exec ${GIT_EXEC_PATH}/bin-wrappers/git $@
Should I do that in a reroll ?

 Two possible alternatives:

  - Is there a reason you would not want to install whatever Perl
modules you want to use via GITPERLLIB mechanism to
../../perl/blib/lib?
 If we are making modifications to Git/Mediawiki.pm or even git-mw.perl
 / git-remote-mediawiki.perl, installing them without proper testing
 beforehand seems wrong.

 That's not installing (i.e. not make install), just a copy within
 the source tree. Same as what's done in Git's toplevel.
Oh right, sorry, read it too quickly :/ Well, I still find it weird to
copy untested files out of contrib/mw-to-git/ though.

Another idea crossed my mind: for now the test suite creates a symlink
of git-remote-mediawiki in the toplevel if it's not installed. It
would be better to use the bin-wrapper in the testsuite I think ?
(plus, with the current solution, it may have problems finding the
latest Git::Mediawiki.pm)
--
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: contrib/mw-to-git/Git/Mediawiki.pm

2013-06-27 Thread Benoît Person
Oops, so sorry :/

It's defintely doable since the lowercase 'git' is only a bin-wrapper
for git to ease development in contrib/mw-to-git/ .

Junio, Matthieu : should I resend a new version of my serie which
renames the 'git' (lowercase) file into something like 'git-dev' ?
(some comments directly mentionning the 'git' (lowercase) file needs
to be updated as well in the Makefile)

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: contrib/mw-to-git/Git/Mediawiki.pm

2013-06-27 Thread Benoît Person
 Hmph.  Does it even need to be in-tree then?  Is it insufficient to
 run ../../git from that directory instead?
Well, the fact is we use Perl packages now (Git.pm and
Git::Mediawiki.pm in contrib/mw-to-git/Git/). The way we build perl
scripts in the toplevel's Makefile makes those packages accessible
only in $GITPERLLIB if it's defined and defaults to $INSTLIBDIR to
seek for installed version of those packages.

We use a bin-wrapper to define that $GITPERLLIB variable so that the
installed version gets bypassed by the one presents in the directory
defined in $GITPERLLIB.

 I just noticed that the script is not strictly a text file, ending
 with an incomplete line, by the way.
an incomplete line ?
--
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 v6 2/5] git-remote-mediawiki: new git bin-wrapper for developement

2013-06-27 Thread Benoît Person
 As far as I can tell, the only real reason why you need this and
 cannot use ../../bin-wrappers/git directly is because the GITPERLLIB
 it gives you only points at ../../perl/blib/lib and not this
 directory.
Plus (forgot to mention it in the other mail :/ ) it enables us to not
copy git-mw and git-remote-mediawiki at each make and stop us from
polluting the toplevel directory with those two scripts during
development.

 Two possible alternatives:

  - Is there a reason you would not want to install whatever Perl
modules you want to use via GITPERLLIB mechanism to
../../perl/blib/lib?
If we are making modifications to Git/Mediawiki.pm or even git-mw.perl
/ git-remote-mediawiki.perl, installing them without proper testing
beforehand seems wrong.

Perhaps it will interfere with the real
installation step in ../../perl/Makefile?
I don't think so, but I am not an expert on what's going on in
../../perl/Makefile.

If that is the case,
then it is not a good idea, but otherwise, that would let you use
../../bin-wrappers/git as-is.

  - Perhaps we could do:

 GITPERLLIB=${GPLEXTRA+$GPLEXTRA:}@@BUILD_DIR@@/perl/blib/lib

in wrap-for-bin.sh, so that your instruction can become

 GPLEXTRA=$(pwd) ../../bin-wrappers/git whatever-mw-thing

and you do not have to have this file?  We would also need to
unset GPLEXTRA at the beginning of test-lib.sh if we were to do
this.


 How does a developer (or user) use this script?  From your Makefile
 (e.g. make test)?  Manually following some written instruction?
 In either case, the latter option feels a lot more sensible
 alternative without having to maintain this extra copy of wrap-for-bin
 that can easily go out of sync.
A developer uses it like any bin-wrapper : bin-wrapper/git mw preview
for instance. He can add it to its path while working on the scripts
etc ...

The best way to do it would be to factorise those new use cases
(adding new elements in $GITPERLLIB and $PATH) in the code that
generate bin-wrappers from wrap-for-bin.sh I think.

But it was weird to work on that in this serie which initial goal was
to add a preview tool for git-remote-mediawiki and ended up adding a
new perl package, a new 'git mw' command which handles subcommands,
...

In the end, this patch could be removed from the serie since it is
self-contained : it is not used by 3/5, 4/5, and 5/5.

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 v4 0/5] git-remote-mediawiki: new tool to preview local changes without pushing

2013-06-20 Thread Benoît Person
Ok, thanks for all the reviews,

Final version (I hope :) ) will come when all the mediawiki patches
will have graduated to 'master' then.

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


Re: [PATCH V3 1/4] git-mw: Introduction of GitMediawiki.pm

2013-06-18 Thread Benoît Person
On 17 June 2013 09:12, Matthieu Moy matthieu@grenoble-inp.fr wrote:
 Also, it seems to be only part of the solution. With your change, from
 contrib/mw-to-git/ and after running only make,

 ./git-mw takes the installed version of GitMediawiki.pm in priority

 ../../bin-wrappers/git takes the installed version of git-mw only (i.e.
 does not know git mw if make install hasn't been ran).
 Same thing as the documentation point, I think I am a bit lost in that
 whole thing. I will re-look into it for the next version :/ .

 In short, the include path should contain both the *.pm file and the
 git-foo ones.
The fact is, for now, is there a way to test changes in
git-remote-mediawiki.perl without 'make install'-ing it ? I could not find one

So maybe in the build-perl-script of the toplevel Makefile we could add
something copying the script at the toplevel ?

And in GitMediawiki's Makefile, we let everything stay as is : copying *.pm
into /perl/blib/lib when building and copying it in installdir when installing ?

 I think you removed a newline from the end of the file. It's usually
 considered good practice to have this trailing newline (e.g. so that
 cat file in a terminal doesn't put your prompt after the last line).
 IIRC, it's actually required to call the file a text file according to
 POSIX.
That catch oO, thanks for the explanations.
From my point of view, this could definitely be improved from:
 - perlcritic -2 *.perl
 + perlcritic -2 *.perl
 \ No newline at end of file
to something like that:
 - perlcritic -2 *.perl
 + perlcritic -2 *.perl
 \ removed newline at end of file
which gives more insights into why the line is considered edited.

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


Re: [PATCH] send-email: allow use of basic email list in --cc --to and --bcc

2013-06-18 Thread benoît person
 +sub split_email_list {
 +my(@list) = @_;
 +my @tmp;
 +my @emails;
 +   for (my $i = 0; $i = $#list; $i++) {
 +   if ($list[$i] =~ /,/) {
 +   @emails = split(/,/, $list[$i]);
 +   } else {
 +   @emails = $list[$i];
 +   }
 +   # Removal of unwanted spaces
 +   for (my $j = 0; $j = $#emails; $j++) {
 +   $emails[$j] =~ s/^\s+//;
 +   $emails[$j] =~ s/\s+$//;
 +   }
 +   @tmp = (@tmp, @emails);
 +   }
 +return(@tmp);
 +}
Why two regex ? You could do something like :
$emails[$j] =~ s/^\s+|\s+$//g;
to remove leading and trailing whitespaces at the same time.

I think it's better to use the builin 'push' function to concatenate
your two arrays:
push(@tmp, @emails);

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


Re: [PATCH V3 1/4] git-mw: Introduction of GitMediawiki.pm

2013-06-16 Thread Benoît Person
On 16 June 2013 22:18, Matthieu Moy matthieu@grenoble-inp.fr wrote:
 benoit.per...@ensimag.fr writes:

 changes from the V2:
   - Add a way to test, without installation, code that uses GitMediawiki.pm.

 This still needs to be documented, even very quickly, somewhere in the
 code (e.g a comment in the Makefile).
Well, I think I will have to re-read some docs (and some earlier
reviews) about what to write in commit messages, in the emails, in the
code as comments and in the documentation ... I am just totally lost
right now :/ .

 -build install clean:
 +copy_pm:
 + cp $(GIT_MEDIAWIKI_PM) $(GIT_ROOT_DIR)/perl/blib/lib/

 I already commented on this:

 http://thread.gmane.org/gmane.comp.version-control.git/227711/focus=227721
Oops, I forgot that one, so sorry :/ .

 Also, it seems to be only part of the solution. With your change, from
 contrib/mw-to-git/ and after running only make,

 ./git-mw takes the installed version of GitMediawiki.pm in priority

 ../../bin-wrappers/git takes the installed version of git-mw only (i.e.
 does not know git mw if make install hasn't been ran).
Same thing as the documentation point, I think I am a bit lost in that
whole thing. I will re-look into it for the next version :/ .

  perlcritic:
 - perlcritic -2 *.perl
 + perlcritic -2 *.perl
 \ No newline at end of file

 Please, avoid these whitespace-only changes. They create noise during
 review, and more potential conflicts.
For that one, I don't know why git assumes there is a change in it :
from what I see there is absolutely no whitespaces changes but maybe
my editor added some weird character somewhere ? I will look into that
for the next version ...

Thank you,

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


Re: [PATCH/RFC 3/4] git-mw: Adding git-mw.perl script

2013-06-15 Thread Benoît Person
The V3 is ready, but I am still not sure about what is the best way to
do it for this issue though.

On 13 June 2013 15:01, Matthieu Moy matthieu@grenoble-inp.fr wrote:
 benoit.per...@ensimag.fr writes:
 How does the make Vs make install work? How does a developer run the
 tool without installing?
Well it does not work without installing but I think you know that now :)

 I first tried:

 $ ../../bin-wrappers/git mw
 git: 'mw' is not a git command. See 'git --help'.

 Then, this first seem OK:

 $ ./git-mw
 usage: git mw command args

 git mw commands are:
 HelpDisplay help information about git mw
 Preview Parse and render local file into HTML

 BUT, this will take the installed GitMediawiki.pm if it is available,
 and we don't want this (if one hacks GitMediawiki.pm locally, one wants
 the new hacked to be taken into account without make installing it).

 To understand better how it works, try adding this in git-mw.perl:

   print $_\n for @INC;

 I get this:

 /home/moy/local/usr-squeeze/share/perl/5.14.2
 /home/moy/local/usr-squeeze/src/MediaWiki-API-0.39/blib/lib
 /etc/perl
 /usr/local/lib/perl/5.14.2
 /usr/local/share/perl/5.14.2
 /usr/lib/perl5
 /usr/share/perl5
 /usr/lib/perl/5.14
 /usr/share/perl/5.14
 /usr/local/lib/site_perl
 .

 The '.' is there, but it comes after the hardcoded
 /home/moy/local/usr-squeeze/share/perl/5.14.2 (which has to comes first,
 to let the install version be robust to whatever comes after).
Thanks for the explanations

 I think you need an equivalent of Git's toplevel bin-wrappers/git, or
 perhaps use the same bin-wrapper/git but let make install in
 contrib/mw-to-git/ install GitMediawiki.pm in perl/blib/lib
Typo s/make install/make/ ?

For now, I have implemented that one : each time you do `make`, if
there is changes in GitMediawiki.pm, it gets copied to $GITPERLLIB
(perl/blib/lib). But I am not sure it's the best approach here. If we
want something entirely self-contained for GitMediawiki, creating a
new git wrapper seems like the best way. But then, we could say that
since GitMediawiki Makefile uses Git toplevel Makefile, it's not
entirely self-contained :/ maybe it's the copying file that makes it
weird ?

 BTW, I just noticed we had a Git::SVN, so perhaps GitMediawiki should be
 Git::MediaWiki.
For that one, I am not really sure Git::Mediawiki makes more sense
than GitMediawiki. The point of the GitMediawiki.pm package is to
contain all the stuff for the bidirectionnal-thingy. So they are not
really Git-related, nor Mediawiki-related. Making it part of a Git
directory / namespace does not really feels right, even if it's how
it's done for SVN :/ .

Thank you for the all the reviews, (it works for Ensiwiki now \o/)

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


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 v3 22/28] git-remote-mediawiki: Modify strings for a better coding-style

2013-06-10 Thread Benoît Person
Well, I think next step would be to replace all those calls with
Git.pm `command`, `command_oneline` and `config``which take an array
of arguments after the command. In the preview tool we use those but
I don't know if we will find the time to clean that up too in
git-remote-mediawiki :) .

Don't know though if it's better to mix that with this serie which is
mainly based on what perlcritic returns.

On 10 June 2013 18:41, Junio C Hamano gits...@pobox.com wrote:
 Matthieu Moy matthieu@grenoble-inp.fr writes:

 Célestin Matte celestin.ma...@ensimag.fr writes:

 @@ -1285,8 +1285,7 @@ sub get_mw_namespace_id {
  # Look at configuration file, if the record for that namespace 
 is
  # already cached. Namespaces are stored in form:
  # Name_of_namespace:Id_namespace, ex.: File:6.
 -my @temp = split(/\n/, run_git(config --get-all remote.
 -. $remotename 
 ..namespaceCache));
 +my @temp = split(/\n/, run_git(config --get-all 
 remote.${remotename}.namespaceCache));

 I tend to prefer the former, as it avoids long lines ( 80 columns)

 But you split the name of a single variable across lines by doing so.

 You could split lines to make it a bit more readable.

 my @temp = split(/\n/,
 run_git(config --get-all 
 remote.${remotename}.namespaceCache));

 It still overflows the 80-column limit, but the namespaceCache is
 the only thing that overflows; not much harm is done.

 This is totally outside of the topic of coding-style series, but I
 would be more concerned about the use of ${remotename}, though.  Has
 it (and in general the parameters to all calls to run_git()) been
 sanitized for shell metacharacters?  If we had a variant of run_git
 that took an array of command line arguments given to git, you could
 do this

 my @temp = split(/\n/,
 run_git([qw(config --get-all),
 remote.${remotename}.namespaceCache)]);

 which would be safer and easier to read.

 @@ -1339,8 +1338,7 @@ sub get_mw_namespace_id {

  # Store explicitely requested namespaces on disk
  if (!exists $cached_mw_namespace_id{$name}) {
 -run_git(config --add remote.. $remotename
 -..namespaceCache \. $name .:. $store_id .\);
 +run_git(qq(config --add remote.${remotename}.namespaceCache 
 ${name}:${store_id}));

 Same.
 --
 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
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 v2 02/22] git-remote-mediawiki: Use the Readonly module instead of the constant pragma

2013-06-08 Thread Benoît Person
The major drawback of using perl `constant` is the fact that they do
not interpolate like variables in most of the contexts (those who
automatically quotes barewords). Like said on Perl Monks here [1], you
have to do some tricks depending on the context in which you're
using the constant and that's not really clean.

But maybe trading clean for another package is not worth it. I just
searched for alternatives way of doing it and none seems to be in the
core packages so maybe we should just drop this.

[1] http://www.perlmonks.org/?node_id=11

On 8 June 2013 05:23, Jeff King p...@peff.net wrote:
 On Fri, Jun 07, 2013 at 11:42:03PM +0200, Célestin Matte wrote:

 diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl 
 b/contrib/mw-to-git/git-remote-mediawiki.perl
 index 4893442..e60793a 100755
 --- a/contrib/mw-to-git/git-remote-mediawiki.perl
 +++ b/contrib/mw-to-git/git-remote-mediawiki.perl
 @@ -26,21 +26,21 @@ use IPC::Open2;
  use Readonly;

 What does this series apply on top of? The existing version in master
 does not have use Readonly in it at all. The first version of your
 series introduced that line, but here it is shown as an existing line.
 Did you miss a commit when putting your patches together?

  # Mediawiki filenames can contain forward slashes. This variable decides by 
 which pattern they should be replaced
 -use constant SLASH_REPLACEMENT = %2F;
 +Readonly my $SLASH_REPLACEMENT = %2F;

 What advantage does this have over use constant? I do not mind
 following guidelines from perlcritic if they are a matter of style, but
 in this case there is a cost: we now depend on the Readonly module,
 which is not part of the standard distribution. I.e., users now have to
 deal with installing an extra dependency. Is it worth 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
--
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


contributing to git-mediawiki

2013-05-31 Thread Benoît Person
Hello,

We are another team from Ensimag (Célestin MATTE  Benoit PERSON) who
will contribute to Git and more specifically to Git-Mediawiki for our
one-month school project - and possibly more. We already have a couple
of basic patches in local and will submit them in the upcoming days.
After that, we will start working on more useful features. As a first
start, we thought about these two ideas posted in the Git-Mediawiki’s
issue tracker:
- A mechanism to preview local modifications in the browser (with
the wiki style) -- https://github.com/moy/Git-Mediawiki/issues/7
- Support for “push-by-rev” to increase push performance on some
wikis -- https://github.com/moy/Git-Mediawiki/issues/6

If you have any suggestions, ideas, guidances, feel free to share them with us.

Best regards,
Célestin MATTE  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