* [PATCH] [RFC] Get subject of current patch in rebase-apply mode [not found] <20161120013325.12113-1-genml+zsh-workers@thequod.de> @ 2016-11-20 1:33 ` Daniel Hahler 2016-11-20 1:43 ` Daniel Hahler ` (3 more replies) 0 siblings, 4 replies; 10+ messages in thread From: Daniel Hahler @ 2016-11-20 1:33 UTC (permalink / raw) To: zsh-workers From: Daniel Hahler <git@thequod.de> --- Functions/VCS_Info/Backends/VCS_INFO_get_data_git | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git index 18ba89a..93172fc 100644 --- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git +++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git @@ -236,6 +236,15 @@ elif [[ -d "${gitdir}/rebase-apply" ]]; then done if [[ -f "${patchdir}/msg-clean" ]]; then subject="${$(< "${patchdir}/msg-clean")[(f)1]}" + else + local maxlines=10 + while IFS='\n' read -r; do + if [[ "$REPLY" == "Subject:"* ]]; then + subject=${REPLY/(#s)Subject: /} + break + fi + (( --maxlines )) || break + done < "${patchdir}/$(printf "%04d" $cur)" fi if [[ -f "${patchdir}/original-commit" ]]; then if [[ -n $subject ]]; then -- 2.10.2 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [RFC] Get subject of current patch in rebase-apply mode 2016-11-20 1:33 ` [PATCH] [RFC] Get subject of current patch in rebase-apply mode Daniel Hahler @ 2016-11-20 1:43 ` Daniel Hahler 2016-11-21 0:16 ` Frank Terbeck ` (2 subsequent siblings) 3 siblings, 0 replies; 10+ messages in thread From: Daniel Hahler @ 2016-11-20 1:43 UTC (permalink / raw) To: Zsh Hackers' List [-- Attachment #1.1: Type: text/plain, Size: 1273 bytes --] Please acknowledge that this patch is fine, before i) commit it. It works for me, but I am unsure about style etc. On 20.11.2016 02:33, Daniel Hahler wrote: > From: Daniel Hahler > > --- > Functions/VCS_Info/Backends/VCS_INFO_get_data_git | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git > index 18ba89a..93172fc 100644 > --- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git > +++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git > @@ -236,6 +236,15 @@ elif [[ -d "${gitdir}/rebase-apply" ]]; then > done > if [[ -f "${patchdir}/msg-clean" ]]; then > subject="${$(< "${patchdir}/msg-clean")[(f)1]}" > + else > + local maxlines=10 > + while IFS='\n' read -r; do > + if [[ "$REPLY" == "Subject:"* ]]; then > + subject=${REPLY/(#s)Subject: /} > + break > + fi > + (( --maxlines )) || break > + done < "${patchdir}/$(printf "%04d" $cur)" > fi > if [[ -f "${patchdir}/original-commit" ]]; then > if [[ -n $subject ]]; then > [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 163 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [RFC] Get subject of current patch in rebase-apply mode 2016-11-20 1:33 ` [PATCH] [RFC] Get subject of current patch in rebase-apply mode Daniel Hahler 2016-11-20 1:43 ` Daniel Hahler @ 2016-11-21 0:16 ` Frank Terbeck 2016-11-22 4:13 ` Daniel Shahaf 2016-11-28 22:56 ` [PATCH] fixup! " Daniel Hahler 3 siblings, 0 replies; 10+ messages in thread From: Frank Terbeck @ 2016-11-21 0:16 UTC (permalink / raw) To: Daniel Hahler; +Cc: zsh-workers Hi, I'd use a named local parameter instead of REPLY and clear out IFS completely: Daniel Hahler wrote: > + local maxlines=10 > + while IFS='\n' read -r; do local maxlines=10 line while IFS= read -r line; do …and then use $line instead of $REPLY in the rest. Regards, Frank ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [RFC] Get subject of current patch in rebase-apply mode 2016-11-20 1:33 ` [PATCH] [RFC] Get subject of current patch in rebase-apply mode Daniel Hahler 2016-11-20 1:43 ` Daniel Hahler 2016-11-21 0:16 ` Frank Terbeck @ 2016-11-22 4:13 ` Daniel Shahaf 2016-11-28 22:59 ` Daniel Hahler 2016-11-28 22:56 ` [PATCH] fixup! " Daniel Hahler 3 siblings, 1 reply; 10+ messages in thread From: Daniel Shahaf @ 2016-11-22 4:13 UTC (permalink / raw) To: Daniel Hahler; +Cc: zsh-workers Daniel Hahler wrote on Sun, Nov 20, 2016 at 02:33:25 +0100: > From: Daniel Hahler <git@thequod.de> > > --- > Functions/VCS_Info/Backends/VCS_INFO_get_data_git | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git > index 18ba89a..93172fc 100644 > --- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git > +++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git > @@ -236,6 +236,15 @@ elif [[ -d "${gitdir}/rebase-apply" ]]; then > done > if [[ -f "${patchdir}/msg-clean" ]]; then > subject="${$(< "${patchdir}/msg-clean")[(f)1]}" I use an older version of git that still creates msg-clean files (see 36830), so I rm'd the msg-clean file by hand to test your patch. > + else > + local maxlines=10 > + while IFS='\n' read -r; do Change '\n' to $'\n'. (But read the last paragraph first.) > + if [[ "$REPLY" == "Subject:"* ]]; then > + subject=${REPLY/(#s)Subject: /} > + break > + fi > + (( --maxlines )) || break > + done < "${patchdir}/$(printf "%04d" $cur)" I'm not an expert on git internal data structures, but I see two failure modes to this patch: the file "$patchdir/${(l:4::0:)cur}" might not exist, or might correspond to a different commit than $cur. The former case could be addressed by adding a [[ -f ]] existence check, as elsewhere in the file. The latter would cause information for a wrong commit to be printed instead of a commit hash. That would be a bug, but it seems unlikely enough. So I would suggest two code changes: use «${(l:4::0:)cur}» or the new 'printf -v' instead of a command substitution to save a fork on Cygwin, and consider adding a [[ -f ]] check to prevent error messages. One last thing I'd suggest is to consider using VCS_INFO_quilt-patch2subject, since it implements rfc822 unfolding of long subject lines. Cheers, Daniel > fi > if [[ -f "${patchdir}/original-commit" ]]; then > if [[ -n $subject ]]; then > -- > 2.10.2 > > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [RFC] Get subject of current patch in rebase-apply mode 2016-11-22 4:13 ` Daniel Shahaf @ 2016-11-28 22:59 ` Daniel Hahler 2016-11-29 6:15 ` Daniel Shahaf 2016-12-01 17:28 ` [PATCH] [RFC] Get subject of current patch in rebase-apply mode Daniel Shahaf 0 siblings, 2 replies; 10+ messages in thread From: Daniel Hahler @ 2016-11-28 22:59 UTC (permalink / raw) To: zsh-workers [-- Attachment #1.1: Type: text/plain, Size: 494 bytes --] On 22.11.2016 05:13, Daniel Shahaf wrote: I've included all your remarks in the fixup commit (just sent), except for: > One last thing I'd suggest is to consider using VCS_INFO_quilt-patch2subject, > since it implements rfc822 unfolding of long subject lines. I think should be applied to the other places then, too - and in the long run being moved into a more general function then. Thanks for the pointer, but I think it is out of scope for this patch. Cheers, Daniel. [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [RFC] Get subject of current patch in rebase-apply mode 2016-11-28 22:59 ` Daniel Hahler @ 2016-11-29 6:15 ` Daniel Shahaf 2016-12-02 23:20 ` Daniel Hahler 2016-12-01 17:28 ` [PATCH] [RFC] Get subject of current patch in rebase-apply mode Daniel Shahaf 1 sibling, 1 reply; 10+ messages in thread From: Daniel Shahaf @ 2016-11-29 6:15 UTC (permalink / raw) To: Daniel Hahler; +Cc: zsh-workers Daniel Hahler wrote on Mon, Nov 28, 2016 at 23:59:18 +0100: > On 22.11.2016 05:13, Daniel Shahaf wrote: > > I've included all your remarks in the fixup commit (just sent), except for: > > > One last thing I'd suggest is to consider using VCS_INFO_quilt-patch2subject, > > since it implements rfc822 unfolding of long subject lines. > > I think should be applied to the other places then, too - and in the > long run being moved into a more general function then. > For future reference — what are those "other places"? > Thanks for the pointer, but I think it is out of scope for this patch. Fair enough; perfect good enemy-of let-be don't. Cheers, Daniel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [RFC] Get subject of current patch in rebase-apply mode 2016-11-29 6:15 ` Daniel Shahaf @ 2016-12-02 23:20 ` Daniel Hahler 2016-12-03 10:34 ` [PATCH] vcs_info git: rfc822-unfold rebase-apply patch subjects when msg-clean is unavailable Daniel Shahaf 0 siblings, 1 reply; 10+ messages in thread From: Daniel Hahler @ 2016-12-02 23:20 UTC (permalink / raw) To: zsh-workers [-- Attachment #1.1: Type: text/plain, Size: 519 bytes --] >>> One last thing I'd suggest is to consider using VCS_INFO_quilt-patch2subject, >>> since it implements rfc822 unfolding of long subject lines. >> >> I think should be applied to the other places then, too - and in the >> long run being moved into a more general function then. > > For future reference — what are those "other places"? The other places (at least one) where a subject gets used. Thanks for refactoring out in 400600. I have committed my patch as-is for now. Cheers, Daniel. [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 195 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] vcs_info git: rfc822-unfold rebase-apply patch subjects when msg-clean is unavailable. 2016-12-02 23:20 ` Daniel Hahler @ 2016-12-03 10:34 ` Daniel Shahaf 0 siblings, 0 replies; 10+ messages in thread From: Daniel Shahaf @ 2016-12-03 10:34 UTC (permalink / raw) To: zsh-workers; +Cc: Daniel Hahler Example (in this repository): git -c merge.merge-changelog.driver=/bin/false rebase --onto=1955cce^^ 1955cce^ 1955cce --- I think this patch covers all the "other places" Daniel referred to. Daniel Functions/VCS_Info/Backends/VCS_INFO_get_data_git | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git index 1560d7f..a92261f 100644 --- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git +++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git @@ -237,14 +237,11 @@ elif [[ -d "${gitdir}/rebase-apply" ]]; then if [[ -f "${patchdir}/msg-clean" ]]; then subject="${$(< "${patchdir}/msg-clean")[(f)1]}" elif [[ -f "${patchdir}/${(l:4::0:)cur}" ]]; then - local maxlines=10 line - while IFS= read -r line; do - if [[ "$line" == "Subject:"* ]]; then - subject=${line/(#s)Subject: /} - break - fi - (( --maxlines )) || break - done < "${patchdir}/${(l:4::0:)cur}" + () { + local REPLY + VCS_INFO_patch2subject "${patchdir}/${(l:4::0:)cur}" + subject=$REPLY + } fi if [[ -f "${patchdir}/original-commit" ]]; then if [[ -n $subject ]]; then @@ -270,6 +267,7 @@ elif [[ -f "${gitdir}/MERGE_HEAD" ]]; then # This is 'git merge --no-commit' local -a heads=( ${(@f)"$(<"${gitdir}/MERGE_HEAD")"} ) local subject; + # TODO: maybe read up to the first blank line IFS='' read -r subject < "${gitdir}/MERGE_MSG" # $subject is the subject line of the would-be commit # Maybe we can get the subject lines of MERGE_HEAD's commits cheaply? @@ -295,6 +293,7 @@ elif [[ -f "${gitdir}/CHERRY_PICK_HEAD" ]]; then # ### be "1". The %u/%c tuple will assume the values [(1,2), (1,1), (1,0)], # ### whereas the correct sequence would be [(1,2), (2,1), (3,0)]. local subject + # TODO: maybe read up to the first blank line IFS='' read -r subject < "${gitdir}/MERGE_MSG" git_patches_applied=( "$(<${gitdir}/CHERRY_PICK_HEAD) ${subject}" ) if [[ -f "${gitdir}/sequencer/todo" ]]; then ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] [RFC] Get subject of current patch in rebase-apply mode 2016-11-28 22:59 ` Daniel Hahler 2016-11-29 6:15 ` Daniel Shahaf @ 2016-12-01 17:28 ` Daniel Shahaf 1 sibling, 0 replies; 10+ messages in thread From: Daniel Shahaf @ 2016-12-01 17:28 UTC (permalink / raw) To: Daniel Hahler; +Cc: zsh-workers Daniel Hahler wrote on Mon, Nov 28, 2016 at 23:59:18 +0100: > On 22.11.2016 05:13, Daniel Shahaf wrote: > > One last thing I'd suggest is to consider using VCS_INFO_quilt-patch2subject, > > since it implements rfc822 unfolding of long subject lines. > > I think should be applied to the other places then, too - and in the long run being moved into a more general function then. > > Thanks for the pointer, but I think it is out of scope for this patch. I've gone ahead and factored that helper function out as a vcs_info-wide helper function in 8891f5a33d26. ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH] fixup! [RFC] Get subject of current patch in rebase-apply mode 2016-11-20 1:33 ` [PATCH] [RFC] Get subject of current patch in rebase-apply mode Daniel Hahler ` (2 preceding siblings ...) 2016-11-22 4:13 ` Daniel Shahaf @ 2016-11-28 22:56 ` Daniel Hahler 3 siblings, 0 replies; 10+ messages in thread From: Daniel Hahler @ 2016-11-28 22:56 UTC (permalink / raw) To: zsh-workers From: Daniel Hahler <git@thequod.de> --- Functions/VCS_Info/Backends/VCS_INFO_get_data_git | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git index 93172fc..1560d7f 100644 --- a/Functions/VCS_Info/Backends/VCS_INFO_get_data_git +++ b/Functions/VCS_Info/Backends/VCS_INFO_get_data_git @@ -236,15 +236,15 @@ elif [[ -d "${gitdir}/rebase-apply" ]]; then done if [[ -f "${patchdir}/msg-clean" ]]; then subject="${$(< "${patchdir}/msg-clean")[(f)1]}" - else - local maxlines=10 - while IFS='\n' read -r; do - if [[ "$REPLY" == "Subject:"* ]]; then - subject=${REPLY/(#s)Subject: /} + elif [[ -f "${patchdir}/${(l:4::0:)cur}" ]]; then + local maxlines=10 line + while IFS= read -r line; do + if [[ "$line" == "Subject:"* ]]; then + subject=${line/(#s)Subject: /} break fi (( --maxlines )) || break - done < "${patchdir}/$(printf "%04d" $cur)" + done < "${patchdir}/${(l:4::0:)cur}" fi if [[ -f "${patchdir}/original-commit" ]]; then if [[ -n $subject ]]; then -- 2.10.2 ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-12-03 10:37 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20161120013325.12113-1-genml+zsh-workers@thequod.de> 2016-11-20 1:33 ` [PATCH] [RFC] Get subject of current patch in rebase-apply mode Daniel Hahler 2016-11-20 1:43 ` Daniel Hahler 2016-11-21 0:16 ` Frank Terbeck 2016-11-22 4:13 ` Daniel Shahaf 2016-11-28 22:59 ` Daniel Hahler 2016-11-29 6:15 ` Daniel Shahaf 2016-12-02 23:20 ` Daniel Hahler 2016-12-03 10:34 ` [PATCH] vcs_info git: rfc822-unfold rebase-apply patch subjects when msg-clean is unavailable Daniel Shahaf 2016-12-01 17:28 ` [PATCH] [RFC] Get subject of current patch in rebase-apply mode Daniel Shahaf 2016-11-28 22:56 ` [PATCH] fixup! " Daniel Hahler
Code repositories for project(s) associated with this public inbox https://git.vuxu.org/mirror/zsh/ This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).