Re: [PATCH v8 3/7] git-remote-mediawiki: New git bin-wrapper for developement
On 5 July 2013 09:04, Matthieu Moy 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 >> # >> +# 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
>> 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
> 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
> 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: [PATCH v6 2/5] git-remote-mediawiki: new git bin-wrapper for developement
> 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: contrib/mw-to-git/Git/Mediawiki.pm
> 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: contrib/mw-to-git/Git/Mediawiki.pm
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: [PATCH v4 0/5] git-remote-mediawiki: new tool to preview local changes without pushing
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] send-email: allow use of basic email list in --cc --to and --bcc
> +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
On 17 June 2013 09:12, Matthieu Moy 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- 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 V3 1/4] git-mw: Introduction of GitMediawiki.pm
On 16 June 2013 22:18, Matthieu Moy 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
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 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 > > 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 install"ing 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
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 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
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 wrote: > Matthieu Moy writes: > >> Célestin Matte 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
On 9 June 2013 08:08, Jeff King 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
Matthieu Moy 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 v2 02/22] git-remote-mediawiki: Use the Readonly module instead of the constant pragma
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 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
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