Re: [PATCH 9/9] trailer: add tests for git interpret-trailers
Christian Couder chrisc...@tuxfamily.org writes: +# Do not remove trailing spaces below! +cat complex_message_trailers 'EOF' +Fixes: +Acked-by: +Reviewed-by: +Signed-off-by: +EOF Just a hint. I think it is far safer and robust over time to do something like this: sed -e 's/ Z$/ /' -\EOF Fixes: Z Acked-by: Z EOF instead of a comment, which can warn human developers but does not do anything to prevent their editors' auto-fix features from kicking in. -- 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 9/9] trailer: add tests for git interpret-trailers
On Mon, Dec 30, 2013 at 09:19:55AM -0800, Junio C Hamano wrote: Christian Couder chrisc...@tuxfamily.org writes: +# Do not remove trailing spaces below! +cat complex_message_trailers 'EOF' +Fixes: +Acked-by: +Reviewed-by: +Signed-off-by: +EOF Just a hint. I think it is far safer and robust over time to do something like this: sed -e 's/ Z$/ /' -\EOF Fixes: Z Acked-by: Z EOF instead of a comment, which can warn human developers but does not do anything to prevent their editors' auto-fix features from kicking in. This, but for simplicity, since every line needs the trailing space, why not just use 's/$/ /' and drop the ' Z' on every line? /bikeshed - Josh Triplett -- 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 9/9] trailer: add tests for git interpret-trailers
Josh Triplett j...@joshtriplett.org writes: On Mon, Dec 30, 2013 at 09:19:55AM -0800, Junio C Hamano wrote: Christian Couder chrisc...@tuxfamily.org writes: +# Do not remove trailing spaces below! +cat complex_message_trailers 'EOF' +Fixes: +Acked-by: +Reviewed-by: +Signed-off-by: +EOF Just a hint. I think it is far safer and robust over time to do something like this: sed -e 's/ Z$/ /' -\EOF Fixes: Z Acked-by: Z EOF instead of a comment, which can warn human developers but does not do anything to prevent their editors' auto-fix features from kicking in. This, but for simplicity, since every line needs the trailing space, why not just use 's/$/ /' and drop the ' Z' on every line? /bikeshed - Josh Triplett A few reasons: - The everybody will have a single SP at the end may or may not last forever; - With your scheme, if you already had _one_ trailing SPs in the input, it would be hard to spot in the source; - It makes it visually very clear that we expect a SP after these colons. This is especially true if you replace 'Z' with something more readable (e.g. '|'). -- 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 9/9] trailer: add tests for git interpret-trailers
On Mon, Dec 30, 2013 at 12:46:33PM -0800, Junio C Hamano wrote: Josh Triplett j...@joshtriplett.org writes: On Mon, Dec 30, 2013 at 09:19:55AM -0800, Junio C Hamano wrote: Christian Couder chrisc...@tuxfamily.org writes: +# Do not remove trailing spaces below! +cat complex_message_trailers 'EOF' +Fixes: +Acked-by: +Reviewed-by: +Signed-off-by: +EOF Just a hint. I think it is far safer and robust over time to do something like this: sed -e 's/ Z$/ /' -\EOF Fixes: Z Acked-by: Z EOF instead of a comment, which can warn human developers but does not do anything to prevent their editors' auto-fix features from kicking in. This, but for simplicity, since every line needs the trailing space, why not just use 's/$/ /' and drop the ' Z' on every line? /bikeshed - Josh Triplett A few reasons: - The everybody will have a single SP at the end may or may not last forever; Trivially fixed if that ever changes, but given the nature of all of these, that seems unlikely. - With your scheme, if you already had _one_ trailing SPs in the input, it would be hard to spot in the source; Git makes them quite difficult to miss. :) - Josh Triplett -- 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 9/9] trailer: add tests for git interpret-trailers
Josh Triplett j...@joshtriplett.org writes: - The everybody will have a single SP at the end may or may not last forever; Trivially fixed if that ever changes, but given the nature of all of these, that seems unlikely. Why? Because we encourage to write tests that are expected to find breakages, some of these test vector lines may have to show a broken line that lacks SP after label + colon. - With your scheme, if you already had _one_ trailing SPs in the input, it would be hard to spot in the source; Git makes them quite difficult to miss. :) That is irrelevant, isn't it? This is about protecting the source in the editor, before you run git show --whitespace=trailing-space, git diff --check, etc. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 9/9] trailer: add tests for git interpret-trailers
Signed-off-by: Christian Couder chrisc...@tuxfamily.org --- t/t7513-interpret-trailers.sh | 206 ++ 1 file changed, 206 insertions(+) create mode 100755 t/t7513-interpret-trailers.sh diff --git a/t/t7513-interpret-trailers.sh b/t/t7513-interpret-trailers.sh new file mode 100755 index 000..beb58fe --- /dev/null +++ b/t/t7513-interpret-trailers.sh @@ -0,0 +1,206 @@ +#!/bin/sh +# +# Copyright (c) 2013 Christian Couder +# + +test_description='git interpret-trailers' + +. ./test-lib.sh + +cat basic_message 'EOF' +subject + +body +EOF + +cat complex_message_body 'EOF' +my subject + +my body which is long +and contains some special +chars like : = ? ! + +EOF + +# Do not remove trailing spaces below! +cat complex_message_trailers 'EOF' +Fixes: +Acked-by: +Reviewed-by: +Signed-off-by: +EOF + +test_expect_success 'without config' ' + printf ack: Peff\nReviewed-by: \nAcked-by: Johan\n expected + git interpret-trailers ack = Peff Reviewed-by Acked-by: Johan actual + test_cmp expected actual +' + +test_expect_success '--trim-empty without config' ' + printf ack: Peff\nAcked-by: Johan\n expected + git interpret-trailers --trim-empty ack = Peff Reviewed-by Acked-by: Johan sob: actual + test_cmp expected actual +' + +test_expect_success 'with config setup' ' + git config trailer.ack.key Acked-by: + printf Acked-by: Peff\n expected + git interpret-trailers --trim-empty ack = Peff actual + test_cmp expected actual + git interpret-trailers --trim-empty Acked-by = Peff actual + test_cmp expected actual + git interpret-trailers --trim-empty Acked-by :Peff actual + test_cmp expected actual +' + +test_expect_success 'with config setup and = sign' ' + git config trailer.ack.key Acked-by= + printf Acked-by= Peff\n expected + git interpret-trailers --trim-empty ack = Peff actual + test_cmp expected actual + git interpret-trailers --trim-empty Acked-by= Peff actual + test_cmp expected actual + git interpret-trailers --trim-empty Acked-by : Peff actual + test_cmp expected actual +' + +test_expect_success 'with config setup and # sign' ' + git config trailer.bug.key Bug # + printf Bug #42\n expected + git interpret-trailers --trim-empty bug = 42 actual + test_cmp expected actual +' + +test_expect_success 'with commit basic message' ' + git interpret-trailers --infile basic_message actual + test_cmp basic_message actual +' + +test_expect_success 'with commit complex message' ' + cat complex_message_body complex_message_trailers complex_message + cat complex_message_body expected + printf Fixes: \nAcked-by= \nReviewed-by: \nSigned-off-by: \n expected + git interpret-trailers --infile complex_message actual + test_cmp expected actual +' + +test_expect_success 'with commit complex message and args' ' + cat complex_message_body expected + printf Fixes: \nAcked-by= \nAcked-by= Peff\nReviewed-by: \nSigned-off-by: \nBug #42\n expected + git interpret-trailers --infile complex_message ack: Peff bug: 42 actual + test_cmp expected actual +' + +test_expect_success 'with commit complex message, args and --trim-empty' ' + cat complex_message_body expected + printf Acked-by= Peff\nBug #42\n expected + git interpret-trailers --trim-empty --infile complex_message ack: Peff bug: 42 actual + test_cmp expected actual +' + +test_expect_success 'using where = before' ' + git config trailer.bug.where before + cat complex_message_body expected + printf Bug #42\nFixes: \nAcked-by= \nAcked-by= Peff\nReviewed-by: \nSigned-off-by: \n expected + git interpret-trailers --infile complex_message ack: Peff bug: 42 actual + test_cmp expected actual +' + +test_expect_success 'using where = before for a token in the middle of infile' ' + git config trailer.review.key Reviewed-by: + git config trailer.review.where before + cat complex_message_body expected + printf Bug #42\nFixes: \nAcked-by= \nAcked-by= Peff\nReviewed-by: Johan\nReviewed-by: \nSigned-off-by: \n expected + git interpret-trailers --infile complex_message ack: Peff bug: 42 review: Johan actual + test_cmp expected actual +' + +test_expect_success 'using where = before and --trim-empty' ' + cat complex_message_body expected + printf Bug #46\nBug #42\nAcked-by= Peff\nReviewed-by: Johan\n expected + git interpret-trailers --infile complex_message --trim-empty ack: Peff bug: 42 review: Johan Bug: 46 actual + test_cmp expected actual +' + +test_expect_success 'the default is ifExist = addIfDifferent' ' + cat complex_message_body expected + printf Bug #42\nFixes: \nAcked-by= \nAcked-by= Peff\nReviewed-by: \nSigned-off-by: \n expected + git
