[-- Attachment #1: Type: text/plain, Size: 1068 bytes --] Dear zsh developers, the subversion functions for tab completion provided by current stable version of zsh (5.7.1) are obsolete and they do not work as expected (e.g. 'svn diff <tab>’ does not provide all files under revision control), since the latest svn versions store all files in the folder .svn of the root directory of the repository. Hence, I patched a bit the original file making svn completion work again but simplifying the logic of tab completion, in that it completes all files and dirs in the current folder (i.e. not only the ones under revision control), best Cristiano ____________ Cristiano De Michele, Ph.D. Department of Physics Tel. : +390649913524 University of Rome "La Sapienza" Fax : +39064463158 Piazzale Aldo Moro, 2 I-00185 Roma - Italy homepage: http://www.roma1.infn.it/~demichel <http://www.roma1.infn.it/~demichel/> [-- Attachment #2.1: Type: text/html, Size: 869 bytes --] [-- Attachment #2.2: _subversion_patched.gz --] [-- Type: application/x-gzip, Size: 5073 bytes --] [-- Attachment #2.3: Type: text/html, Size: 2864 bytes --]
Cristiano De Michele wrote on Fri, Aug 23, 2019 at 18:38:07 +0200: > the subversion functions for tab completion provided by current stable > version of zsh (5.7.1) are obsolete and they do not work as expected > (e.g. 'svn diff <tab>’ does not provide all files under revision > control), I can't reproduce this. What files does it not complete? > simplifying the logic of tab completion, in that it completes all > files and dirs in the current folder (i.e. not only the ones under > revision control), Note that completion should also complete files that are missing (status «!»), scheduled for deletion (status «D»), and in the case of «svn up», files that are cropped (after an earlier «svn up --set-depth=exclude»). Does your patch regress either of these cases? (If multiple things are broken, a patch needs to fix only one of them to be acceptable — provided that it doesn't regress anything else. If it does, it will need to be considered on a case-by-case basis.) > since the latest svn versions store all files in the folder > .svn of the root directory of the repository. Context for the list: Subversion through 1.6.x put .svn dirs in each and every subdirectory, like CVS did. Subversion ≥1.7 put one .svn dir in the working copy root, like git and hg do. Subversion 1.7.0 was released in Oct 2011. Subversion 1.6.x EOL'd in June 2013. At this point I see little value in continuing to support the ≤1.6 working copy format in zsh. If anyone disagrees, speak up. > Hence, I patched a bit the original file making svn completion work > again but Thanks for basing your work on HEAD of master. Please send patches as unified diffs, not as full files. That's easier to read and review. Non parlo italiano. If the comment at the top of the file is pertinent, please translate it to English. Why did you delete _svn_conflicts()? The pattern there is still valid. I'm not fond of deleting _svn_deletedfiles(), _svn_controlled(), and _svn_status(). If there's no way to fix them, I'd prefer to keep them as functions that return true, so it would be easier to restore the functionality once someone figures out an implementation. _svn_status() could probably be salvaged just by making it check the mtime of wc.db and wc.db-journal instead (more or less; IIRC SQLite has some pragmas/options affecting naming of auxiliary files). _svn_deletedfiles(), by using «svn st | grep '^D' | cut -c8-». _svn_controlled(), by using «svn info -R» or «svn st --xml». (It would be wrong to use «svn ls» there: that wouldn't DTRT on mixed-revision and/or switched working copies.) And looking further into it, _svn_deletedfiles() and _svn_controlled() can both be computed by a _single_ command: «svn info -R --show-item=schedule». Thanks for the patch! Cheers, Daniel
[-- Attachment #1: Type: text/plain, Size: 5024 bytes --] Dear Daniel, first of all thank you for the prompt reply. To reproduce the issue try to do what follows. Let's assume that there is a file called 'rescheduler.py' and a directory called 'RESCHEDULE_EXAMPLE' in a svn repository (both under version control), then > svn diff r<TAB> does not complete it as expected, since it suggests only the directory 'RESCHEDULE_EXAMPLE'. The same odd behavior can be also experienced with 'log' argument (i.e. svn log). With my patch it should provide all possible completions (i.e. all files and disr in the folder), since I removed completely the smart logic, which was present in the original functions (i.e. I removed everywhere the -g option and the relative argument of function _file) I also attempted to reimplement the functions _svn_status and _svn_controlled to restore the original logic as follows (( $+functions[_svn_controlled] )) || _svn_controlled() { sta=$(svn status $REPLY) stat=$sta[1] [[ $stat != "?" ]] } (( $+functions[_svn_status] )) || _svn_status() { sta=$(svn status $REPLY) [[ "$sta" != "" ]] } but I am not sure they work as the original versions… I agree with you that _svn_conflict could be kept. According to what you wrote a possible implementation of _svn_deletedfiles could be sta=$(svn status $REPLY) stat=$sta[1] [[ $stat == “D" ]] what do you think? The patch file is attached, cheers Cristiano ____________ Cristiano De Michele, Ph.D. Department of Physics Tel. : +390649913524 University of Rome "La Sapienza" Fax : +39064463158 Piazzale Aldo Moro, 2 I-00185 Roma - Italy homepage: http://www.roma1.infn.it/~demichel <http://www.roma1.infn.it/~demichel> > Il giorno 24 ago 2019, alle ore 19:24, Daniel Shahaf <d.s@daniel.shahaf.name> ha scritto: > > Cristiano De Michele wrote on Fri, Aug 23, 2019 at 18:38:07 +0200: >> the subversion functions for tab completion provided by current stable >> version of zsh (5.7.1) are obsolete and they do not work as expected >> (e.g. 'svn diff <tab>’ does not provide all files under revision >> control), > > I can't reproduce this. What files does it not complete? > >> simplifying the logic of tab completion, in that it completes all >> files and dirs in the current folder (i.e. not only the ones under >> revision control), > > Note that completion should also complete files that are missing (status > «!»), scheduled for deletion (status «D»), and in the case of «svn up», > files that are cropped (after an earlier «svn up --set-depth=exclude»). > Does your patch regress either of these cases? > > (If multiple things are broken, a patch needs to fix only one of them to > be acceptable — provided that it doesn't regress anything else. If it > does, it will need to be considered on a case-by-case basis.) > >> since the latest svn versions store all files in the folder >> .svn of the root directory of the repository. > > Context for the list: > > Subversion through 1.6.x put .svn dirs in each and every subdirectory, > like CVS did. Subversion ≥1.7 put one .svn dir in the working copy > root, like git and hg do. Subversion 1.7.0 was released in Oct 2011. > Subversion 1.6.x EOL'd in June 2013. At this point I see little value > in continuing to support the ≤1.6 working copy format in zsh. If anyone > disagrees, speak up. > >> Hence, I patched a bit the original file making svn completion work >> again but > > Thanks for basing your work on HEAD of master. Please send patches > as unified diffs, not as full files. That's easier to read and review. > > Non parlo italiano. If the comment at the top of the file is pertinent, > please translate it to English. > > Why did you delete _svn_conflicts()? The pattern there is still valid. > > I'm not fond of deleting _svn_deletedfiles(), _svn_controlled(), and > _svn_status(). If there's no way to fix them, I'd prefer to keep them > as functions that return true, so it would be easier to restore the > functionality once someone figures out an implementation. > > _svn_status() could probably be salvaged just by making it check the > mtime of wc.db and wc.db-journal instead (more or less; IIRC SQLite has > some pragmas/options affecting naming of auxiliary files). > > _svn_deletedfiles(), by using «svn st | grep '^D' | cut -c8-». > > _svn_controlled(), by using «svn info -R» or «svn st --xml». (It would > be wrong to use «svn ls» there: that wouldn't DTRT on mixed-revision > and/or switched working copies.) > > And looking further into it, _svn_deletedfiles() and _svn_controlled() > can both be computed by a _single_ command: «svn info -R > --show-item=schedule». > > Thanks for the patch! > > Cheers, > > Daniel [-- Attachment #2.1: Type: text/html, Size: 7736 bytes --] [-- Attachment #2.2: _subversion_patch.gz --] [-- Type: application/x-gzip, Size: 1218 bytes --] [-- Attachment #2.3: Type: text/html, Size: 6877 bytes --]
Cristiano De Michele wrote on Sat, 24 Aug 2019 20:26 +00:00: > Dear Daniel, > first of all thank you for the prompt reply. To reproduce the issue try > to do what follows. > Let's assume that there is a file called 'rescheduler.py' and a > directory > called 'RESCHEDULE_EXAMPLE' in a svn repository (both under version > control), then > > svn diff r<TAB> > does not complete it as expected, since it suggests only the directory > 'RESCHEDULE_EXAMPLE'. Interesting. I can reproduce this in a new repository that contains just these two entries, but if I go to one of my usual working copies and modify some files, those files do get offered by the completion. > The same odd behavior can be also experienced with 'log' argument > (i.e. svn log). Okay, but note that 'diff' and 'log' would be expected to complete different things. 'diff' tries to complete only files with local mods; 'log' should complete only filenames that exist on the server. For example: Status diff? log? ------ ----- ---- A yes no M yes yes <space> no yes > With my patch it should provide all possible completions (i.e. all > files and disr in the folder), since I removed completely the smart > logic, > > which was present in the original functions (i.e. I removed everywhere > the -g option and the relative argument of function _file) As I said in my previous email, simply using _files wouldn't complete everything that should be completed: files with status '!' or 'D', or files with 'Depth: exclude', for example. > (( $+functions[_svn_controlled] )) || > _svn_controlled() { > sta=$(svn status $REPLY) > stat=$sta[1] > [[ $stat != "?" ]] > } This sounds right, though the machinery currently in there (_call_program, cache) should be added back. Calling 'svn st foo' for each file individually would be slow. Also, a '--' should be added. > (( $+functions[_svn_status] )) || > _svn_status() { > > sta=$(svn status $REPLY) > [[ "$sta" != "" ]] > } Not sure. What about conflicted files? Unmodified files in a changelist? Unmodified files that were locked by another working copy? > but I am not sure they work as the original versions… > I agree with you that _svn_conflict could be kept. According > to what you wrote a possible implementation of _svn_deletedfiles could be > > sta=$(svn status $REPLY) > stat=$sta[1] > [[ $stat == “D" ]] What about completing foo/bar after «svn rm foo»? > what do you think? Thanks a lot for working on this. Let's try to break this into small, incremental improvements. > The patch file is attached, You sent the diff reversed: the file as in HEAD of master should be the first argument to diff. Also, for bonus points, send the diff with a .txt extension and not compressed. (Doing so gives it a text/* MIME type, which makes the diff easier to read on our end.) Cheers, Daniel
[-- Attachment #1: Type: text/plain, Size: 850 bytes --] Dear Daniel, attached you can find the diff file which you requested, cheers C. ____________ Cristiano De Michele, Ph.D. Department of Physics Tel. : +390649913524 University of Rome "La Sapienza" Fax : +39064463158 Piazzale Aldo Moro, 2 I-00185 Roma - Italy homepage: http://www.roma1.infn.it/~demichel <http://www.roma1.infn.it/~demichel/> > Il giorno 24 ago 2019, alle ore 23:56, Daniel Shahaf <d.s@daniel.shahaf.name> ha scritto: > > Cristiano De Michele wrote on Sat, 24 Aug 2019 21:41 +00:00: >> attached you can find the diff file which you requested, > > Thanks! Yes, that's the right form to use in the future. > [-- Attachment #2.1: Type: text/html, Size: 409 bytes --] [-- Attachment #2.2: subversion_patch.txt --] [-- Type: text/plain, Size: 3473 bytes --] --- _subversion_original_zsh_5.7.1 2019-08-24 09:54:06.000000000 +0200 +++ _subversion_orthogonal 2019-08-24 22:10:57.000000000 +0200 @@ -1,5 +1,9 @@ #compdef svn svnlite=svn svnadmin svnadmin-static=svnadmin +# This is the file provided by zsh 5.7.1 patched in such a way that I removed from all the calls +# to _file zsh function the option -g and the relative argument, thus making completion less smart +# with compared to original version. + _svn () { local curcontext="$curcontext" state line expl ret=1 typeset -A opt_args @@ -106,23 +110,23 @@ case $cmd in; (add) args+=( - '*:file:_files -g "*(^e:_svn_controlled:)"' + '*:file:_files' ) ;; (commit) args=( ${args/(#b)(*--file*):arg:/$match[1]:file:_files} - '*:file:_files -g "*(e:_svn_status:)"' + '*:file:_files' ) ;; (delete) args+=( - '*:file:_files -g ".svn(/e:_svn_deletedfiles:)"' + '*:file:_files' ) ;; (diff) args+=( - '*: : _alternative "files:file:_files -g \*\(e:_svn_status:\)" "urls:URL:_svn_urls"' + '*: : _alternative "files:file:_files" "urls:URL:_svn_urls"' ) ;; (help) @@ -138,8 +142,8 @@ ;; (log) args+=( - '1: : _alternative "files:file:_files -g \*\(e:_svn_controlled:\)" "urls:URL:_svn_urls"' - '*:file:_files -g "*(e:_svn_controlled:)"' + '1: : _alternative "files:file:_files" "urls:URL:_svn_urls"' + '*:file:_files' ) ;; (mergeinfo) @@ -161,12 +165,12 @@ ;; (resolved) args+=( - '*:file:_files -g "*(e:_svn_conflicts:)"' + '*:file:_files' ) ;; (revert) args+=( - '*:file:_files -g "(.svn|*)(/e:_svn_deletedfiles:,e:_svn_status:)"' + '*:file:_files' ) ;; (unshelve) @@ -308,44 +312,6 @@ return ret } -(( $+functions[_svn_controlled] )) || -_svn_controlled() { - [[ -f ${(M)REPLY##*/}.svn/text-base/${REPLY##*/}.svn-base ]] -} - -(( $+functions[_svn_conflicts] )) || -_svn_conflicts() { - [ -n $REPLY.(mine|r<->)(N[1]) ] -} - -(( $+functions[_svn_deletedfiles] )) || -_svn_deletedfiles() { - # Typical usage would be _files -g '.svn(/e:_svn_deletedfiles:)' - local cont controlled - reply=( ) - [[ $REPLY = (*/|).svn ]] || return - controlled=( $REPLY/text-base/*.svn-base(N:r:t) ) - for cont in ${controlled}; do - [[ -e $REPLY:h/$cont ]] || reply+=( ${REPLY%.svn}$cont ) - done -} - -(( $+functions[_svn_status] )) || -_svn_status() { - local dir=$REPLY:h - local pat="${1:-([ADMR~]|?M)}" - - zmodload -F zsh/stat b:zstat 2>/dev/null - local key="$(zstat +device $dir):$(zstat +inode $dir)" - local mtime="$(zstat +mtime $dir/.svn/entries)" - - if (( ! $+_cache_svn_status[$key] || _cache_svn_mtime[$key] != mtime )); then - _cache_svn_status[$key]="$(_call_program files svn status -N $dir)" - _cache_svn_mtime[$key]="$mtime" - fi - - (( ${(M)#${(f)_cache_svn_status[$key]}:#(#s)${~pat}*$REPLY} )) -} (( $+functions[_svn_remote_paths] )) || _svn_remote_paths() { [-- Attachment #2.3: Type: text/html, Size: 3471 bytes --]
Cristiano De Michele wrote on Sun, 25 Aug 2019 09:46 +00:00:
> attached you can find the diff file which you requested,
Thanks, appreciated!
You really didn't need to re-send it so many times over, though. My
comments about the patch format were just "for future reference"; I
didn't mean to make you jump through syntactic hoops.
Cheers,
Daniel