[PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Dmitry Kurochkin]

On Wed, 15 Jun 2011 10:00:36 -0700, Carl Worth  wrote:
> On Wed, 15 Jun 2011 18:25:14 +0400, Dmitry Kurochkin  gmail.com> wrote:
> > I know you prefer tests to go before patches and I agree with that.
> 
> Great!
> 
> > But most of the time I do tests after coding.
> 
> Yes, I do that order almost exclusively as well.
> 
> > I do not know an easy way to reorder patches in git.  (Also I do not
> > know how to amend an old patch
> 
> Fortunately, git has a great feature here for both use cases, (git
> rebase -i). Here's the simple recipe:
> 
> * Find a bug, fix a bug, commit
> 
> * Write a test case, commit
> 
> * Run the following command:
> 
>   git rebase -i origin/master
> 
> At this point you'll be presented with an editor window giving one line
> for each commit that you have made since origin/master. You can reorder
> these lines however you'd like. When you save and exit the editor, the
> commits will be applied in the order you saved.
> 
> If there are any conflicts due to the re-ordering, then git rebase will
> stop and tell you what to do, which will be:
> 
> * Resolve the conflict
> 
> * Run "git add" on the files you edited
> 
> * Run "git rebase --continue"
> 
> Also, back when editing the original list of commits, you can change the
> word "apply" next to any particular commit to change what happens when
> applying it. If you change that to "reword" you'll be given an editor
> window to edit the commit message. If you use "edit" then you'll be
> dropped to a shell where you can:
> 
> * Edit the code
> 
> * Test as necessary
> 
> * Run "git commit --amend"
> 
> * Run "git rebase --continue"
> 
> I absolutely love "git rebase -i". It's one of my favorite
> user-interface features in git.
> 

Thanks for this.  I did not know about interactive mode in rebase.

This is some sort of replacement for darcs amend (which allows editing
any patch, not just the last one).

> > wish more darcs features in git.
> 
> I don't know about "git rebase -i", but I think I heard that "git add
> -i", (interactively add some portions of the dirty working tree to the
> index to be committed). I think the menu-based interface of "git add -i"
> is particularly clunky. But I love the trimmed-down interface of "git
> add -p" which simply prompts one-patch-hunk-at-a-time for pieces to add
> to the next commit. It even supports splitting a hunk, (or even manually
> editing the patch to trim it down!). It's pretty slick stuff.
> 

Yes, "add -i" is ugl... confusing, but "add -p" is very nice.  A great
feature of darcs picked up by git.

> So there are some git tips that might be useful.
> 

They will be useful indeed.  Thanks!

Regards,
  Dmitry

> > Thanks.
> 
> You're quite welcome. Thanks for all the great work. Please keep it up!
> 
> -Carl
> 
> -- 
> carl.d.worth at intel.com

[PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Dmitry Kurochkin]

On Wed, 15 Jun 2011 07:06:40 -0700, Carl Worth  wrote:
> On Fri, 27 May 2011 01:42:22 +0400, Dmitry Kurochkin  gmail.com> wrote:
> > I have just send two more patches to this thread.  One with new tests.
> > Another with a workaround for the bug.  The workaround should not break
> > anything since it affects only notmuch-show mode.
> 
> Thanks!
> 
> I love committing tests that demonstrate broken code before committing
> fixes. As it happened here, I committed these two new patches thinking I
> had previously committed the earlier patches in the series. Fortunately,
> the failure of the test pointed out that I was missing the actual fix.
> 

I know you prefer tests to go before patches and I agree with that.  But
most of the time I do tests after coding.  I do not know an easy way to
reorder patches in git.  (Also I do not know how to amend an old patch,
wish more darcs features in git.)  Hopefully it is not a big trouble for
you to reorder the patches when applying.

> I also really like the workaround to avoid regressing functionality
> because of an emacs bug.
> 

indeed

> Well done, Dmitry. I've now pushed out everything in this series.
> 

Thanks.

Regards,
  Dmitry

> -Carl
> 
> -- 
> carl.d.worth at intel.com

Re: [PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Dmitry Kurochkin]

On Wed, 15 Jun 2011 10:00:36 -0700, Carl Worth  wrote:
> On Wed, 15 Jun 2011 18:25:14 +0400, Dmitry Kurochkin 
>  wrote:
> > I know you prefer tests to go before patches and I agree with that.
> 
> Great!
> 
> > But most of the time I do tests after coding.
> 
> Yes, I do that order almost exclusively as well.
> 
> > I do not know an easy way to reorder patches in git.  (Also I do not
> > know how to amend an old patch
> 
> Fortunately, git has a great feature here for both use cases, (git
> rebase -i). Here's the simple recipe:
> 
> * Find a bug, fix a bug, commit
> 
> * Write a test case, commit
> 
> * Run the following command:
> 
>   git rebase -i origin/master
> 
> At this point you'll be presented with an editor window giving one line
> for each commit that you have made since origin/master. You can reorder
> these lines however you'd like. When you save and exit the editor, the
> commits will be applied in the order you saved.
> 
> If there are any conflicts due to the re-ordering, then git rebase will
> stop and tell you what to do, which will be:
> 
> * Resolve the conflict
> 
> * Run "git add" on the files you edited
> 
> * Run "git rebase --continue"
> 
> Also, back when editing the original list of commits, you can change the
> word "apply" next to any particular commit to change what happens when
> applying it. If you change that to "reword" you'll be given an editor
> window to edit the commit message. If you use "edit" then you'll be
> dropped to a shell where you can:
> 
> * Edit the code
> 
> * Test as necessary
> 
> * Run "git commit --amend"
> 
> * Run "git rebase --continue"
> 
> I absolutely love "git rebase -i". It's one of my favorite
> user-interface features in git.
> 

Thanks for this.  I did not know about interactive mode in rebase.

This is some sort of replacement for darcs amend (which allows editing
any patch, not just the last one).

> > wish more darcs features in git.
> 
> I don't know about "git rebase -i", but I think I heard that "git add
> -i", (interactively add some portions of the dirty working tree to the
> index to be committed). I think the menu-based interface of "git add -i"
> is particularly clunky. But I love the trimmed-down interface of "git
> add -p" which simply prompts one-patch-hunk-at-a-time for pieces to add
> to the next commit. It even supports splitting a hunk, (or even manually
> editing the patch to trim it down!). It's pretty slick stuff.
> 

Yes, "add -i" is ugl... confusing, but "add -p" is very nice.  A great
feature of darcs picked up by git.

> So there are some git tips that might be useful.
> 

They will be useful indeed.  Thanks!

Regards,
  Dmitry

> > Thanks.
> 
> You're quite welcome. Thanks for all the great work. Please keep it up!
> 
> -Carl
> 
> -- 
> carl.d.wo...@intel.com
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Carl Worth]

On Wed, 15 Jun 2011 18:25:14 +0400, Dmitry Kurochkin 
 wrote:
> I know you prefer tests to go before patches and I agree with that.

Great!

> But most of the time I do tests after coding.

Yes, I do that order almost exclusively as well.

> I do not know an easy way to reorder patches in git.  (Also I do not
> know how to amend an old patch

Fortunately, git has a great feature here for both use cases, (git
rebase -i). Here's the simple recipe:

* Find a bug, fix a bug, commit

* Write a test case, commit

* Run the following command:

git rebase -i origin/master

At this point you'll be presented with an editor window giving one line
for each commit that you have made since origin/master. You can reorder
these lines however you'd like. When you save and exit the editor, the
commits will be applied in the order you saved.

If there are any conflicts due to the re-ordering, then git rebase will
stop and tell you what to do, which will be:

* Resolve the conflict

* Run "git add" on the files you edited

* Run "git rebase --continue"

Also, back when editing the original list of commits, you can change the
word "apply" next to any particular commit to change what happens when
applying it. If you change that to "reword" you'll be given an editor
window to edit the commit message. If you use "edit" then you'll be
dropped to a shell where you can:

* Edit the code

* Test as necessary

* Run "git commit --amend"

* Run "git rebase --continue"

I absolutely love "git rebase -i". It's one of my favorite
user-interface features in git.

> wish more darcs features in git.

I don't know about "git rebase -i", but I think I heard that "git add
-i", (interactively add some portions of the dirty working tree to the
index to be committed). I think the menu-based interface of "git add -i"
is particularly clunky. But I love the trimmed-down interface of "git
add -p" which simply prompts one-patch-hunk-at-a-time for pieces to add
to the next commit. It even supports splitting a hunk, (or even manually
editing the patch to trim it down!). It's pretty slick stuff.

So there are some git tips that might be useful.

> Thanks.

You're quite welcome. Thanks for all the great work. Please keep it up!

-Carl

-- 
carl.d.wo...@intel.com


pgpbqJ9DsetYl.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Carl Worth]

On Wed, 15 Jun 2011 18:25:14 +0400, Dmitry Kurochkin  wrote:
> I know you prefer tests to go before patches and I agree with that.

Great!

> But most of the time I do tests after coding.

Yes, I do that order almost exclusively as well.

> I do not know an easy way to reorder patches in git.  (Also I do not
> know how to amend an old patch

Fortunately, git has a great feature here for both use cases, (git
rebase -i). Here's the simple recipe:

* Find a bug, fix a bug, commit

* Write a test case, commit

* Run the following command:

git rebase -i origin/master

At this point you'll be presented with an editor window giving one line
for each commit that you have made since origin/master. You can reorder
these lines however you'd like. When you save and exit the editor, the
commits will be applied in the order you saved.

If there are any conflicts due to the re-ordering, then git rebase will
stop and tell you what to do, which will be:

* Resolve the conflict

* Run "git add" on the files you edited

* Run "git rebase --continue"

Also, back when editing the original list of commits, you can change the
word "apply" next to any particular commit to change what happens when
applying it. If you change that to "reword" you'll be given an editor
window to edit the commit message. If you use "edit" then you'll be
dropped to a shell where you can:

* Edit the code

* Test as necessary

* Run "git commit --amend"

* Run "git rebase --continue"

I absolutely love "git rebase -i". It's one of my favorite
user-interface features in git.

> wish more darcs features in git.

I don't know about "git rebase -i", but I think I heard that "git add
-i", (interactively add some portions of the dirty working tree to the
index to be committed). I think the menu-based interface of "git add -i"
is particularly clunky. But I love the trimmed-down interface of "git
add -p" which simply prompts one-patch-hunk-at-a-time for pieces to add
to the next commit. It even supports splitting a hunk, (or even manually
editing the patch to trim it down!). It's pretty slick stuff.

So there are some git tips that might be useful.

> Thanks.

You're quite welcome. Thanks for all the great work. Please keep it up!

-Carl

-- 
carl.d.worth at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 

Re: [PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Jameson Graef Rollins]

On Wed, 15 Jun 2011 18:25:14 +0400, Dmitry Kurochkin 
 wrote:
> I know you prefer tests to go before patches and I agree with that.  But
> most of the time I do tests after coding.  I do not know an easy way to
> reorder patches in git.  (Also I do not know how to amend an old patch,
> wish more darcs features in git.)  Hopefully it is not a big trouble for
> you to reorder the patches when applying.

Hey, Dmitry.  I usually just pop in to a new branch rooted before my new
patch series, and then cherry-pick the new patches onto the branch in
the order I want (eg. tests first, etc.).

jamie.


pgpaAiRVF0L52.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Jameson Graef Rollins]

On Wed, 15 Jun 2011 18:25:14 +0400, Dmitry Kurochkin  wrote:
> I know you prefer tests to go before patches and I agree with that.  But
> most of the time I do tests after coding.  I do not know an easy way to
> reorder patches in git.  (Also I do not know how to amend an old patch,
> wish more darcs features in git.)  Hopefully it is not a big trouble for
> you to reorder the patches when applying.

Hey, Dmitry.  I usually just pop in to a new branch rooted before my new
patch series, and then cherry-pick the new patches onto the branch in
the order I want (eg. tests first, etc.).

jamie.
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: 

Re: [PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Dmitry Kurochkin]

On Wed, 15 Jun 2011 07:06:40 -0700, Carl Worth  wrote:
> On Fri, 27 May 2011 01:42:22 +0400, Dmitry Kurochkin 
>  wrote:
> > I have just send two more patches to this thread.  One with new tests.
> > Another with a workaround for the bug.  The workaround should not break
> > anything since it affects only notmuch-show mode.
> 
> Thanks!
> 
> I love committing tests that demonstrate broken code before committing
> fixes. As it happened here, I committed these two new patches thinking I
> had previously committed the earlier patches in the series. Fortunately,
> the failure of the test pointed out that I was missing the actual fix.
> 

I know you prefer tests to go before patches and I agree with that.  But
most of the time I do tests after coding.  I do not know an easy way to
reorder patches in git.  (Also I do not know how to amend an old patch,
wish more darcs features in git.)  Hopefully it is not a big trouble for
you to reorder the patches when applying.

> I also really like the workaround to avoid regressing functionality
> because of an emacs bug.
> 

indeed

> Well done, Dmitry. I've now pushed out everything in this series.
> 

Thanks.

Regards,
  Dmitry

> -Carl
> 
> -- 
> carl.d.wo...@intel.com
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Carl Worth]

On Fri, 27 May 2011 01:42:22 +0400, Dmitry Kurochkin 
 wrote:
> I have just send two more patches to this thread.  One with new tests.
> Another with a workaround for the bug.  The workaround should not break
> anything since it affects only notmuch-show mode.

Thanks!

I love committing tests that demonstrate broken code before committing
fixes. As it happened here, I committed these two new patches thinking I
had previously committed the earlier patches in the series. Fortunately,
the failure of the test pointed out that I was missing the actual fix.

I also really like the workaround to avoid regressing functionality
because of an emacs bug.

Well done, Dmitry. I've now pushed out everything in this series.

-Carl

-- 
carl.d.wo...@intel.com


pgpJBJjGfTyfI.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Carl Worth]

On Fri, 27 May 2011 01:42:22 +0400, Dmitry Kurochkin  wrote:
> I have just send two more patches to this thread.  One with new tests.
> Another with a workaround for the bug.  The workaround should not break
> anything since it affects only notmuch-show mode.

Thanks!

I love committing tests that demonstrate broken code before committing
fixes. As it happened here, I committed these two new patches thinking I
had previously committed the earlier patches in the series. Fortunately,
the failure of the test pointed out that I was missing the actual fix.

I also really like the workaround to avoid regressing functionality
because of an emacs bug.

Well done, Dmitry. I've now pushed out everything in this series.

-Carl

-- 
carl.d.worth at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 

[PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Dmitry Kurochkin]

On Thu, 26 May 2011 14:31:30 -0700, Carl Worth  wrote:
> On Thu, 26 May 2011 14:26:34 +0400, Dmitry Kurochkin  gmail.com> wrote:
> > It is called indirectly.  What is the best way to fix it?  I imagine
> > that we can replace `isearch-range-invisible' function with another one,
> > which would call the fixed function if some special variable is set, or
> > if we are searching in a notmuch-show view.  Otherwise it would call the
> > original function.
> 
> I'm not sure what the best approach would be. I'd like to be a good
> "emacs citizen" but I don't know what the best way to do that is here.
> 

Agreed on being a good "emacs citizen".

> If we can predict what the first emacs release will be that contains the
> fix, then we could restrict our kludge here to older, unfixed
> emacs. That would help avoid some problems with "bad citizenship here".
> 
> I'm willing to take whatever you this is best here.
> 

I have just send two more patches to this thread.  One with new tests.
Another with a workaround for the bug.  The workaround should not break
anything since it affects only notmuch-show mode.

Regards,
  Dmitry

> -Carl
> 
> -- 
> carl.d.worth at intel.com
Non-text part: application/pgp-signature

Re: [PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Dmitry Kurochkin]

On Thu, 26 May 2011 14:31:30 -0700, Carl Worth  wrote:
> On Thu, 26 May 2011 14:26:34 +0400, Dmitry Kurochkin 
>  wrote:
> > It is called indirectly.  What is the best way to fix it?  I imagine
> > that we can replace `isearch-range-invisible' function with another one,
> > which would call the fixed function if some special variable is set, or
> > if we are searching in a notmuch-show view.  Otherwise it would call the
> > original function.
> 
> I'm not sure what the best approach would be. I'd like to be a good
> "emacs citizen" but I don't know what the best way to do that is here.
> 

Agreed on being a good "emacs citizen".

> If we can predict what the first emacs release will be that contains the
> fix, then we could restrict our kludge here to older, unfixed
> emacs. That would help avoid some problems with "bad citizenship here".
> 
> I'm willing to take whatever you this is best here.
> 

I have just send two more patches to this thread.  One with new tests.
Another with a workaround for the bug.  The workaround should not break
anything since it affects only notmuch-show mode.

Regards,
  Dmitry

> -Carl
> 
> -- 
> carl.d.wo...@intel.com
Non-text part: application/pgp-signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Carl Worth]

On Thu, 26 May 2011 14:26:34 +0400, Dmitry Kurochkin 
 wrote:
> It is called indirectly.  What is the best way to fix it?  I imagine
> that we can replace `isearch-range-invisible' function with another one,
> which would call the fixed function if some special variable is set, or
> if we are searching in a notmuch-show view.  Otherwise it would call the
> original function.

I'm not sure what the best approach would be. I'd like to be a good
"emacs citizen" but I don't know what the best way to do that is here.

If we can predict what the first emacs release will be that contains the
fix, then we could restrict our kludge here to older, unfixed
emacs. That would help avoid some problems with "bad citizenship here".

I'm willing to take whatever you this is best here.

-Carl

-- 
carl.d.wo...@intel.com


pgpJoldixvzPn.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Carl Worth]

On Thu, 26 May 2011 14:26:34 +0400, Dmitry Kurochkin  wrote:
> It is called indirectly.  What is the best way to fix it?  I imagine
> that we can replace `isearch-range-invisible' function with another one,
> which would call the fixed function if some special variable is set, or
> if we are searching in a notmuch-show view.  Otherwise it would call the
> original function.

I'm not sure what the best approach would be. I'd like to be a good
"emacs citizen" but I don't know what the best way to do that is here.

If we can predict what the first emacs release will be that contains the
fix, then we could restrict our kludge here to older, unfixed
emacs. That would help avoid some problems with "bad citizenship here".

I'm willing to take whatever you this is best here.

-Carl

-- 
carl.d.worth at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 

[PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Dmitry Kurochkin]

On Wed, 25 May 2011 18:02:49 -0700, Carl Worth  wrote:
> On Thu, 26 May 2011 03:10:11 +0400, Dmitry Kurochkin  gmail.com> wrote:
> > On Wed, 25 May 2011 15:46:40 -0700, Carl Worth  wrote:
> > Well, emacs trunk is not broken :)  The bug is in lisp code, so you can
> > fix it in .emacs by redefining `isearch-range-invisible' function.  I do
> > that now.
> 
> Oh, in that case we can fix this is notmuch emacs lisp by just defining
> and using a fixed function. Is the broken function something we're
> calling directly? Or is it being called indirectly? (being called by
> other emacs lisp code that we are calling)?
> 

It is called indirectly.  What is the best way to fix it?  I imagine
that we can replace `isearch-range-invisible' function with another one,
which would call the fixed function if some special variable is set, or
if we are searching in a notmuch-show view.  Otherwise it would call the
original function.

> If we can incorporate the fix, that would be great.
> 
> > Please consider pushing other patches from the series.  They do not fix
> > any bug, but do simplify the code.  The last patch uses list for
> > invisible overlay property as well.  But it does not break isearch
> > because we do not search in hidden messages.
> 
> Hmmm... we should probably do that. I'd like isearch in notmuch to
> search anything that is hidden.
> 

That is easy to fix.  I can do that once we are done with this stuff.

> > BTW would be nice to have a set of known-to-fail tests, i.e. bugs that
> > are not fixed yet.  If we had it, the above test could be implemented
> > and committed before we have the fix pushed.
> 
> We do! Use test_expect_equal_failure (yes, the name is horrible!)
> instead of test_expect_equal and you should get what you want.
> 

I should look at it, thanks.

Regards,
  Dmitry

> -Carl
> 
> -- 
> carl.d.worth at intel.com

Re: [PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Dmitry Kurochkin]

On Wed, 25 May 2011 18:02:49 -0700, Carl Worth  wrote:
> On Thu, 26 May 2011 03:10:11 +0400, Dmitry Kurochkin 
>  wrote:
> > On Wed, 25 May 2011 15:46:40 -0700, Carl Worth  wrote:
> > Well, emacs trunk is not broken :)  The bug is in lisp code, so you can
> > fix it in .emacs by redefining `isearch-range-invisible' function.  I do
> > that now.
> 
> Oh, in that case we can fix this is notmuch emacs lisp by just defining
> and using a fixed function. Is the broken function something we're
> calling directly? Or is it being called indirectly? (being called by
> other emacs lisp code that we are calling)?
> 

It is called indirectly.  What is the best way to fix it?  I imagine
that we can replace `isearch-range-invisible' function with another one,
which would call the fixed function if some special variable is set, or
if we are searching in a notmuch-show view.  Otherwise it would call the
original function.

> If we can incorporate the fix, that would be great.
> 
> > Please consider pushing other patches from the series.  They do not fix
> > any bug, but do simplify the code.  The last patch uses list for
> > invisible overlay property as well.  But it does not break isearch
> > because we do not search in hidden messages.
> 
> Hmmm... we should probably do that. I'd like isearch in notmuch to
> search anything that is hidden.
> 

That is easy to fix.  I can do that once we are done with this stuff.

> > BTW would be nice to have a set of known-to-fail tests, i.e. bugs that
> > are not fixed yet.  If we had it, the above test could be implemented
> > and committed before we have the fix pushed.
> 
> We do! Use test_expect_equal_failure (yes, the name is horrible!)
> instead of test_expect_equal and you should get what you want.
> 

I should look at it, thanks.

Regards,
  Dmitry

> -Carl
> 
> -- 
> carl.d.wo...@intel.com
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Dmitry Kurochkin]

On Wed, 25 May 2011 15:46:40 -0700, Carl Worth  wrote:
> On Thu, 26 May 2011 02:34:28 +0400, Dmitry Kurochkin  gmail.com> wrote:
> > I am not sure how it is best to test this.  The common `printc' method
> > for emacs tests does not work, because it prints invisible parts as
> > well.  We need either to find a way to print only visible text on the
> > console, or test it inside emacs somehow.  Any suggestions?
> 
> Unfortunately, I don't have a good plan here. I delayed implementing any
> automated testing at all of the emacs interface precisely because of
> this problem. It's seems to me that surely emacs must have some built-in
> mechanism for copying the visible portion of a block of text, but I've
> not been able to find it.
> 

Me too.

> We could do something cheesy (and slow) by marching through the buffer
> character-by-character in elisp and testing for visibility, but the
> emacs tests are already the slowest part of "make test"[*] so that would
> be obnoxious.
> 

Indeed.

> > Note that this is exactly the patch that hits the isearch emacs bug.  Do
> > I understand correctly that you are ready to push the series despite of
> > it (given that we have a test)?
> 
> Breaking isearch would be really unfortunate. That's a really nice
> feature of the emacs frontend currently.
> 
> So I would notice that breakage, (while I've apparently never before
> noticed the breakage of having visible citations in a hidden message).
> 
> So no, I'm not saying I'm ready to push the series while emacs is broken.
> 

Well, emacs trunk is not broken :)  The bug is in lisp code, so you can
fix it in .emacs by redefining `isearch-range-invisible' function.  I do
that now.

I do not think I will make myself work on the test until it is likely to
be pushed.  I will try to not to forget about it, so sometime later I
may be back to it :)

Please consider pushing other patches from the series.  They do not fix
any bug, but do simplify the code.  The last patch uses list for
invisible overlay property as well.  But it does not break isearch
because we do not search in hidden messages.

BTW would be nice to have a set of known-to-fail tests, i.e. bugs that
are not fixed yet.  If we had it, the above test could be implemented
and committed before we have the fix pushed.

> -Carl
> 
> [*] Maybe the performance of the emacs testing could be significantly
> improved by sharing a single invocation of emacs? Perhaps this wouldn't
> even be hard by just using emacsclient?
> 

This is possible as long as tests do not affect each other.  Would be a
nice improvement.

Regards,
  Dmitry

[PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Dmitry Kurochkin]

On Wed, 25 May 2011 15:23:47 -0700, Carl Worth  wrote:
> On Thu, 26 May 2011 02:10:14 +0400, Dmitry Kurochkin  gmail.com> wrote:
> > Before the change, message and citation invisibility overlays
> > conflicted: if some citation is made visible and then the whole
> > message is hidden, that citation remained visible.
> 
> That sounds like quite a bug. I'd love to see this series also add a
> test case for that.
> 

I am not sure how it is best to test this.  The common `printc' method
for emacs tests does not work, because it prints invisible parts as
well.  We need either to find a way to print only visible text on the
console, or test it inside emacs somehow.  Any suggestions?

Note that this is exactly the patch that hits the isearch emacs bug.  Do
I understand correctly that you are ready to push the series despite of
it (given that we have a test)?

Regards,
  Dmitry

> -Carl
> 
> -- 
> carl.d.worth at intel.com
Non-text part: application/pgp-signature

[PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Dmitry Kurochkin]

Before the change, message and citation invisibility overlays
conflicted: if some citation is made visible and then the whole
message is hidden, that citation remained visible.  This happened
because the citation's overlay has an invisible property which
takes priority over the message overlay.  The message
invisibility spec does not affect citation visibility, it is
determined solely by the citation overlay invisibility spec.
Hence, if citation is made visible, it is not hidden by message
invisibility spec.

The patch changes citation overlay invisibility property to be a
list which contains both the citation and the message
invisibility specs.  This makes the citation invisible if either
of them is added to the `buffer-invisibility-spec'.  Note that
all citation visibility states are "restored" when the message
hidden and shown again.
---
 emacs/notmuch-wash.el |   12 +++-
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
index 7d87e85..bd521ba 100644
--- a/emacs/notmuch-wash.el
+++ b/emacs/notmuch-wash.el
@@ -107,7 +107,8 @@ collapse the remaining lines into a button.")
   :supertype 'notmuch-wash-button-invisibility-toggle-type)

 (defun notmuch-wash-region-isearch-show (overlay)
-  (remove-from-invisibility-spec (overlay-get overlay 'invisible)))
+  (dolist (invis-spec (overlay-get overlay 'invisible))
+(remove-from-invisibility-spec invis-spec)))

 (defun notmuch-wash-button-label (overlay)
   (let* ((type (overlay-get overlay 'type))
@@ -118,7 +119,7 @@ collapse the remaining lines into a button.")
 (lines-count (count-lines (overlay-start overlay) (overlay-end 
overlay
 (format label-format lines-count)))

-(defun notmuch-wash-region-to-button (beg end type prefix)
+(defun notmuch-wash-region-to-button (msg beg end type prefix)
   "Auxilary function to do the actual making of overlays and buttons

 BEG and END are buffer locations. TYPE should a string, either
@@ -131,11 +132,12 @@ insert before the button, probably for indentation."
   ;; since the newly created symbol has no plist.

   (let ((overlay (make-overlay beg end))
+   (message-invis-spec (plist-get msg :message-invis-spec))
(invis-spec (make-symbol (concat "notmuch-" type "-region")))
(button-type (intern-soft (concat "notmuch-wash-button-"
  type "-toggle-type"
 (add-to-invisibility-spec invis-spec)
-(overlay-put overlay 'invisible invis-spec)
+(overlay-put overlay 'invisible (list invis-spec message-invis-spec))
 (overlay-put overlay 'isearch-open-invisible 
#'notmuch-wash-region-isearch-show)
 (overlay-put overlay 'type type)
 (goto-char (1+ end))
@@ -166,7 +168,7 @@ insert before the button, probably for indentation."
  (goto-char cite-end)
  (forward-line (- notmuch-wash-citation-lines-suffix))
  (notmuch-wash-region-to-button
-  hidden-start (point-marker)
+  msg hidden-start (point-marker)
   "citation" "\n")
   (if (and (not (eobp))
   (re-search-forward notmuch-wash-signature-regexp nil t))
@@ -180,7 +182,7 @@ insert before the button, probably for indentation."
  (set-marker sig-end-marker (point-max))
  (overlay-put (make-overlay sig-start-marker sig-end-marker) 'face 
'message-cited-text)
  (notmuch-wash-region-to-button
-  sig-start-marker sig-end-marker
+  msg sig-start-marker sig-end-marker
   "signature" "\n"))

 ;;
-- 
1.7.5.1

Re: [PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Carl Worth]

On Thu, 26 May 2011 03:10:11 +0400, Dmitry Kurochkin 
 wrote:
> On Wed, 25 May 2011 15:46:40 -0700, Carl Worth  wrote:
> Well, emacs trunk is not broken :)  The bug is in lisp code, so you can
> fix it in .emacs by redefining `isearch-range-invisible' function.  I do
> that now.

Oh, in that case we can fix this is notmuch emacs lisp by just defining
and using a fixed function. Is the broken function something we're
calling directly? Or is it being called indirectly? (being called by
other emacs lisp code that we are calling)?

If we can incorporate the fix, that would be great.

> Please consider pushing other patches from the series.  They do not fix
> any bug, but do simplify the code.  The last patch uses list for
> invisible overlay property as well.  But it does not break isearch
> because we do not search in hidden messages.

Hmmm... we should probably do that. I'd like isearch in notmuch to
search anything that is hidden.

> BTW would be nice to have a set of known-to-fail tests, i.e. bugs that
> are not fixed yet.  If we had it, the above test could be implemented
> and committed before we have the fix pushed.

We do! Use test_expect_equal_failure (yes, the name is horrible!)
instead of test_expect_equal and you should get what you want.

-Carl

-- 
carl.d.wo...@intel.com


pgp1vAI5Cj5J5.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Carl Worth]

On Thu, 26 May 2011 03:10:11 +0400, Dmitry Kurochkin  wrote:
> On Wed, 25 May 2011 15:46:40 -0700, Carl Worth  wrote:
> Well, emacs trunk is not broken :)  The bug is in lisp code, so you can
> fix it in .emacs by redefining `isearch-range-invisible' function.  I do
> that now.

Oh, in that case we can fix this is notmuch emacs lisp by just defining
and using a fixed function. Is the broken function something we're
calling directly? Or is it being called indirectly? (being called by
other emacs lisp code that we are calling)?

If we can incorporate the fix, that would be great.

> Please consider pushing other patches from the series.  They do not fix
> any bug, but do simplify the code.  The last patch uses list for
> invisible overlay property as well.  But it does not break isearch
> because we do not search in hidden messages.

Hmmm... we should probably do that. I'd like isearch in notmuch to
search anything that is hidden.

> BTW would be nice to have a set of known-to-fail tests, i.e. bugs that
> are not fixed yet.  If we had it, the above test could be implemented
> and committed before we have the fix pushed.

We do! Use test_expect_equal_failure (yes, the name is horrible!)
instead of test_expect_equal and you should get what you want.

-Carl

-- 
carl.d.worth at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 

Re: [PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Dmitry Kurochkin]

On Wed, 25 May 2011 15:46:40 -0700, Carl Worth  wrote:
> On Thu, 26 May 2011 02:34:28 +0400, Dmitry Kurochkin 
>  wrote:
> > I am not sure how it is best to test this.  The common `printc' method
> > for emacs tests does not work, because it prints invisible parts as
> > well.  We need either to find a way to print only visible text on the
> > console, or test it inside emacs somehow.  Any suggestions?
> 
> Unfortunately, I don't have a good plan here. I delayed implementing any
> automated testing at all of the emacs interface precisely because of
> this problem. It's seems to me that surely emacs must have some built-in
> mechanism for copying the visible portion of a block of text, but I've
> not been able to find it.
> 

Me too.

> We could do something cheesy (and slow) by marching through the buffer
> character-by-character in elisp and testing for visibility, but the
> emacs tests are already the slowest part of "make test"[*] so that would
> be obnoxious.
> 

Indeed.

> > Note that this is exactly the patch that hits the isearch emacs bug.  Do
> > I understand correctly that you are ready to push the series despite of
> > it (given that we have a test)?
> 
> Breaking isearch would be really unfortunate. That's a really nice
> feature of the emacs frontend currently.
> 
> So I would notice that breakage, (while I've apparently never before
> noticed the breakage of having visible citations in a hidden message).
> 
> So no, I'm not saying I'm ready to push the series while emacs is broken.
> 

Well, emacs trunk is not broken :)  The bug is in lisp code, so you can
fix it in .emacs by redefining `isearch-range-invisible' function.  I do
that now.

I do not think I will make myself work on the test until it is likely to
be pushed.  I will try to not to forget about it, so sometime later I
may be back to it :)

Please consider pushing other patches from the series.  They do not fix
any bug, but do simplify the code.  The last patch uses list for
invisible overlay property as well.  But it does not break isearch
because we do not search in hidden messages.

BTW would be nice to have a set of known-to-fail tests, i.e. bugs that
are not fixed yet.  If we had it, the above test could be implemented
and committed before we have the fix pushed.

> -Carl
> 
> [*] Maybe the performance of the emacs testing could be significantly
> improved by sharing a single invocation of emacs? Perhaps this wouldn't
> even be hard by just using emacsclient?
> 

This is possible as long as tests do not affect each other.  Would be a
nice improvement.

Regards,
  Dmitry
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Carl Worth]

On Thu, 26 May 2011 02:34:28 +0400, Dmitry Kurochkin 
 wrote:
> I am not sure how it is best to test this.  The common `printc' method
> for emacs tests does not work, because it prints invisible parts as
> well.  We need either to find a way to print only visible text on the
> console, or test it inside emacs somehow.  Any suggestions?

Unfortunately, I don't have a good plan here. I delayed implementing any
automated testing at all of the emacs interface precisely because of
this problem. It's seems to me that surely emacs must have some built-in
mechanism for copying the visible portion of a block of text, but I've
not been able to find it.

We could do something cheesy (and slow) by marching through the buffer
character-by-character in elisp and testing for visibility, but the
emacs tests are already the slowest part of "make test"[*] so that would
be obnoxious.

> Note that this is exactly the patch that hits the isearch emacs bug.  Do
> I understand correctly that you are ready to push the series despite of
> it (given that we have a test)?

Breaking isearch would be really unfortunate. That's a really nice
feature of the emacs frontend currently.

So I would notice that breakage, (while I've apparently never before
noticed the breakage of having visible citations in a hidden message).

So no, I'm not saying I'm ready to push the series while emacs is broken.

-Carl

[*] Maybe the performance of the emacs testing could be significantly
improved by sharing a single invocation of emacs? Perhaps this wouldn't
even be hard by just using emacsclient?



pgp6iFku77j2A.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Carl Worth]

On Thu, 26 May 2011 02:34:28 +0400, Dmitry Kurochkin  wrote:
> I am not sure how it is best to test this.  The common `printc' method
> for emacs tests does not work, because it prints invisible parts as
> well.  We need either to find a way to print only visible text on the
> console, or test it inside emacs somehow.  Any suggestions?

Unfortunately, I don't have a good plan here. I delayed implementing any
automated testing at all of the emacs interface precisely because of
this problem. It's seems to me that surely emacs must have some built-in
mechanism for copying the visible portion of a block of text, but I've
not been able to find it.

We could do something cheesy (and slow) by marching through the buffer
character-by-character in elisp and testing for visibility, but the
emacs tests are already the slowest part of "make test"[*] so that would
be obnoxious.

> Note that this is exactly the patch that hits the isearch emacs bug.  Do
> I understand correctly that you are ready to push the series despite of
> it (given that we have a test)?

Breaking isearch would be really unfortunate. That's a really nice
feature of the emacs frontend currently.

So I would notice that breakage, (while I've apparently never before
noticed the breakage of having visible citations in a hidden message).

So no, I'm not saying I'm ready to push the series while emacs is broken.

-Carl

[*] Maybe the performance of the emacs testing could be significantly
improved by sharing a single invocation of emacs? Perhaps this wouldn't
even be hard by just using emacsclient?

-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 

Re: [PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Dmitry Kurochkin]

On Wed, 25 May 2011 15:23:47 -0700, Carl Worth  wrote:
> On Thu, 26 May 2011 02:10:14 +0400, Dmitry Kurochkin 
>  wrote:
> > Before the change, message and citation invisibility overlays
> > conflicted: if some citation is made visible and then the whole
> > message is hidden, that citation remained visible.
> 
> That sounds like quite a bug. I'd love to see this series also add a
> test case for that.
> 

I am not sure how it is best to test this.  The common `printc' method
for emacs tests does not work, because it prints invisible parts as
well.  We need either to find a way to print only visible text on the
console, or test it inside emacs somehow.  Any suggestions?

Note that this is exactly the patch that hits the isearch emacs bug.  Do
I understand correctly that you are ready to push the series despite of
it (given that we have a test)?

Regards,
  Dmitry

> -Carl
> 
> -- 
> carl.d.wo...@intel.com
Non-text part: application/pgp-signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Carl Worth]

On Thu, 26 May 2011 02:10:14 +0400, Dmitry Kurochkin 
 wrote:
> Before the change, message and citation invisibility overlays
> conflicted: if some citation is made visible and then the whole
> message is hidden, that citation remained visible.

That sounds like quite a bug. I'd love to see this series also add a
test case for that.

-Carl

-- 
carl.d.wo...@intel.com


pgpZT6IzLLblE.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Carl Worth]

On Thu, 26 May 2011 02:10:14 +0400, Dmitry Kurochkin  wrote:
> Before the change, message and citation invisibility overlays
> conflicted: if some citation is made visible and then the whole
> message is hidden, that citation remained visible.

That sounds like quite a bug. I'd love to see this series also add a
test case for that.

-Carl

-- 
carl.d.worth at intel.com
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 197 bytes
Desc: not available
URL: 

[PATCH 3/5] Fix hiding a message while some citations are shown in notmuch-show view.

[Dmitry Kurochkin]

Before the change, message and citation invisibility overlays
conflicted: if some citation is made visible and then the whole
message is hidden, that citation remained visible.  This happened
because the citation's overlay has an invisible property which
takes priority over the message overlay.  The message
invisibility spec does not affect citation visibility, it is
determined solely by the citation overlay invisibility spec.
Hence, if citation is made visible, it is not hidden by message
invisibility spec.

The patch changes citation overlay invisibility property to be a
list which contains both the citation and the message
invisibility specs.  This makes the citation invisible if either
of them is added to the `buffer-invisibility-spec'.  Note that
all citation visibility states are "restored" when the message
hidden and shown again.
---
 emacs/notmuch-wash.el |   12 +++-
 1 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/emacs/notmuch-wash.el b/emacs/notmuch-wash.el
index 7d87e85..bd521ba 100644
--- a/emacs/notmuch-wash.el
+++ b/emacs/notmuch-wash.el
@@ -107,7 +107,8 @@ collapse the remaining lines into a button.")
   :supertype 'notmuch-wash-button-invisibility-toggle-type)
 
 (defun notmuch-wash-region-isearch-show (overlay)
-  (remove-from-invisibility-spec (overlay-get overlay 'invisible)))
+  (dolist (invis-spec (overlay-get overlay 'invisible))
+(remove-from-invisibility-spec invis-spec)))
 
 (defun notmuch-wash-button-label (overlay)
   (let* ((type (overlay-get overlay 'type))
@@ -118,7 +119,7 @@ collapse the remaining lines into a button.")
 (lines-count (count-lines (overlay-start overlay) (overlay-end 
overlay
 (format label-format lines-count)))
 
-(defun notmuch-wash-region-to-button (beg end type prefix)
+(defun notmuch-wash-region-to-button (msg beg end type prefix)
   "Auxilary function to do the actual making of overlays and buttons
 
 BEG and END are buffer locations. TYPE should a string, either
@@ -131,11 +132,12 @@ insert before the button, probably for indentation."
   ;; since the newly created symbol has no plist.
 
   (let ((overlay (make-overlay beg end))
+   (message-invis-spec (plist-get msg :message-invis-spec))
(invis-spec (make-symbol (concat "notmuch-" type "-region")))
(button-type (intern-soft (concat "notmuch-wash-button-"
  type "-toggle-type"
 (add-to-invisibility-spec invis-spec)
-(overlay-put overlay 'invisible invis-spec)
+(overlay-put overlay 'invisible (list invis-spec message-invis-spec))
 (overlay-put overlay 'isearch-open-invisible 
#'notmuch-wash-region-isearch-show)
 (overlay-put overlay 'type type)
 (goto-char (1+ end))
@@ -166,7 +168,7 @@ insert before the button, probably for indentation."
  (goto-char cite-end)
  (forward-line (- notmuch-wash-citation-lines-suffix))
  (notmuch-wash-region-to-button
-  hidden-start (point-marker)
+  msg hidden-start (point-marker)
   "citation" "\n")
   (if (and (not (eobp))
   (re-search-forward notmuch-wash-signature-regexp nil t))
@@ -180,7 +182,7 @@ insert before the button, probably for indentation."
  (set-marker sig-end-marker (point-max))
  (overlay-put (make-overlay sig-start-marker sig-end-marker) 'face 
'message-cited-text)
  (notmuch-wash-region-to-button
-  sig-start-marker sig-end-marker
+  msg sig-start-marker sig-end-marker
   "signature" "\n"))
 
 ;;
-- 
1.7.5.1

___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch