magit-todos icon indicating copy to clipboard operation
magit-todos copied to clipboard

Next/previous-section sometimes skips sections

Open mrcnski opened this issue 5 years ago • 7 comments

Encountered this little issue while fixing #64 .

If I set (setq magit-todos-update nil), so that I see "update manually" on the magit status screen, the n and p keys no longer work correctly and jump to sections out of order.

I don't know how to fix this so I'll leave it to you, Elder One.

mrcnski avatar Jan 12 '19 14:01 mrcnski

I sometimes see similar behavior, but I haven't been able to track it down. I don't think it depends on the value of magit-todos-update, but I'm not sure. We have to use some hacks since magit-section isn't intended to be used after-the-fact like this.

alphapapa avatar Jan 13 '19 01:01 alphapapa

I'm noticing jumping around when going through the magit-status sections with sections included by forge.

image

With this setup and pressing n from where the point is at on the screenshot:

  1. Unstaged changes
  2. Stashes
  3. Recent commits
  4. (Jumps) TODOs
  5. Pull requests
  6. Issues

Almost like the magit-todos-insert-at setting of 'recent is being used for navigation purposes?

blaenk avatar Jan 24 '19 22:01 blaenk

I think it has something to do with jumping to (point-max) when magit-todos-insert-at is bottom -- I don't think magit-todos-insert-at can work for the purpose for which it was intended.

This should be handled by manipulating the location of the TODOs section within magit-status-sections-hook -- not by playing with point.

This is just my impression after ~10 minutes of investigation though, so take it with a grain of salt.

I'll note that I can reliably reproduce this with default forge/magit-todos configuration.

vermiculus avatar Jan 27 '19 14:01 vermiculus

I think that magit-section does some "internal bookkeeping" to keep track of sections, and the method we're using to insert the section doesn't interact with that, so it becomes out of sync sometimes.

@vermiculus

This should be handled by manipulating the location of the TODOs section within magit-status-sections-hook -- not by playing with point.

That's a very interesting idea. You may be right. If that works, it would probably be a simple solution. I'll look into that when I have time. Thanks.

alphapapa avatar Jan 29 '19 09:01 alphapapa

I've attempted a fix in https://github.com/alphapapa/magit-todos/commit/b60520a6b167b3236e0155e06f4d6fc11fd3c2c8. It seems to fix the problem of skipping sections, however a minor problem remains: that magit-section-forward wraps back to the first section instead of signaling "No next section".

@vermiculus According to magit-insert-section, it inserts a section at point. It doesn't say that anything special is required depending on where point is to ensure proper sequencing with forward/backward commands, so it's possible this minor bug is in magit-section rather than here--depending on one's perspective, anyway. Further investigation may reveal the problem, but that will probably require tracing code in magit-section.el, so I'll leave that for another time. Help wanted. :)

BTW, everyone: please let me know if this fixes the skipping bug for you.

alphapapa avatar Dec 30 '19 07:12 alphapapa

Just got back to this. 😅

BTW, everyone: please let me know if this fixes the skipping bug for you.

It seems to fix it, thanks. But now when I set

(setq magit-todos-update nil)

it shows:

TODOs (branched from master) (0) (update manually)

Wrote https://github.com/alphapapa/magit-todos/issues/122 to follow up.

mrcnski avatar Nov 16 '21 23:11 mrcnski

The skipping of sections seems to still be an issue.

Uploading a new screenshot for reference: Screen Shot 2021-11-22 at 18 08 13 The bug happens when my point is on "TODOs" and I press n.

mrcnski avatar Nov 23 '21 00:11 mrcnski