Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strange branched from master section #122

Closed
mrcnski opened this issue Nov 16, 2021 · 14 comments
Closed

Strange branched from master section #122

mrcnski opened this issue Nov 16, 2021 · 14 comments
Labels
discussion invalid This doesn't seem right

Comments

@mrcnski
Copy link
Contributor

mrcnski commented Nov 16, 2021

When I set

(setq magit-todos-update nil)

it shows a new extra branched from master section:

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

I am profoundly preplexed and deeply disturbed.

@alphapapa
Copy link
Owner

What branch is checked out?

@mrcnski
Copy link
Contributor Author

mrcnski commented Nov 28, 2021

I'm getting it for https://github.com/SkynetLabs/skynet-js, branch revision-cache.

@alphapapa
Copy link
Owner

I'm not sure what to do about this. Please report whether this still happens with the latest version.

@alphapapa alphapapa added help wanted Extra attention is needed WAITING Waiting for response from someone discussion labels Aug 26, 2023
@mrcnski
Copy link
Contributor Author

mrcnski commented Aug 27, 2023

Thanks for circling back to this. Looks like I still get it for that branch. Were you unable to reproduce?

I noticed the latest commit in that branch is a merge from master, that probably has something to do with it?

@alphapapa
Copy link
Owner

alphapapa commented Aug 27, 2023

I can't reproduce it. Here's what I see:

mtt

@mrcnski
Copy link
Contributor Author

mrcnski commented Aug 28, 2023

Here's my view:

Screenshot 2023-08-28 at 10 45 33

More info (magit-todos is the latest from Melpa):

Magit 20230827.1215 [>= 3.3.0.50-git], Transient 0.4.3, Git 2.41.0, Emacs 29.1, darwin

@mrcnski
Copy link
Contributor Author

mrcnski commented Aug 28, 2023

You don't have the update manually text, did you set this?

(setq magit-todos-update nil)

@alphapapa
Copy link
Owner

No, I didn't set that. And now that I've noticed that, I think there's no bug here. Since you're blocking automatic updating of the lists, the section doesn't know whether any items exist to be displayed, so like the non-branched section, it informs the user that manual update is required.

@alphapapa alphapapa added invalid This doesn't seem right and removed help wanted Extra attention is needed WAITING Waiting for response from someone labels Aug 28, 2023
@mrcnski
Copy link
Contributor Author

mrcnski commented Aug 29, 2023

Should there be two TODOs section? The first is always at 0 even after I update, the second says branched from master (not sure why this message has to be there).

@alphapapa
Copy link
Owner

The way I see it, when you disable automatic updates, the sections tell you that they may not be up-to-date. Otherwise, when items do exist but aren't shown due to not being updated, you might not realize that something is missing. In other words, showing the empty section with the reminder prevents false negatives.

You could argue that showing both sections that way is redundant, but I would argue that I don't want to add a special case and make the code more complicated, especially since it only happens when automatic updates are disabled, which AFAICT is rare.

Maybe we should talk about why you've disabled automatic updates. If we could solve that, this issue would go away for you, right?

@mrcnski
Copy link
Contributor Author

mrcnski commented Aug 30, 2023

I don't mean the update manually message, that is self-explanatory. I only mentioned it because it was missing from your screenshot, but it's not the problem.

I mean there are two TODOs sections. The first one seems totally non-functional, always showing 0 todos, and the second says branched from master. The branched from master message has no value to me as a user, and it has nothing to do with todos in the repo. As a user I just want one TODOs section that shows all the todos in the project (after I update manually) without extra noise.

Maybe we should talk about why you've disabled automatic updates.

Updates incur noticeable lag in large repos, even with rg.

@alphapapa
Copy link
Owner

I mean there are two TODOs sections. The first one seems totally non-functional, always showing 0 todos, and the second says branched from master. The branched from master message has no value to me as a user, and it has nothing to do with todos in the repo. As a user I just want one TODOs section that shows all the todos in the project (after I update manually) without extra noise.

I'm confused. I'm getting the impression that you're misunderstanding the purpose of the "branched from master" section. You do realize that it only appears when in non-master (or main, whatever the default branch is) branches, right?

Updates incur noticeable lag in large repos, even with rg.

Ok, how large is "large"? And how many items are we talking about? And what platform are you on?

@mrcnski
Copy link
Contributor Author

mrcnski commented Aug 31, 2023

Yeah, could be a misunderstanding on my part. And apologies for the unclear original issue. So, there being two TODOs sections is not a bug, but intentional, to have a separate section for master as well as the current branch?

I assumed it was a bug due to this section only appearing with (setq magit-todos-update nil) AFAICT. If it's intentional, I can offer a couple of suggestions for improving the user experience:

  1. Name the two sections TODOs (master) and TODOs (branched from master)
  2. For the first section, say update manually in master.

Either of these would have made it more clear to me and avoided this issue. At any rate, I appreciate your patience in sorting this whole mess out.

Ok, how large is "large"? And how many items are we talking about? And what platform are you on?

I don't use magit-todos anymore, so maybe the lag is no longer an issue. This is a good example of a big repo.

@alphapapa
Copy link
Owner

Yeah, could be a misunderstanding on my part. And apologies for the unclear original issue. So, there being two TODOs sections is not a bug, but intentional, to have a separate section for master as well as the current branch?

Yes, one section shows only todos in the checked-out, non-master branch.

I assumed it was a bug due to this section only appearing with (setq magit-todos-update nil) AFAICT. If it's intentional, I can offer a couple of suggestions for improving the user experience:

  1. Name the two sections TODOs (master) and TODOs (branched from master)

That could be done, but I don't think it's necessary, because out of the apparently thousands of users this package has, no one has ever complained about that before. As well, it would make the main section more verbose by default.

  1. For the first section, say update manually in master.

I don't think that would help; I don't even understand what it means. As well, my impression is that few users disable magit-todos-update.

Either of these would have made it more clear to me and avoided this issue. At any rate, I appreciate your patience in sorting this whole mess out.

I appreciate your feedback, although I wish I had known sooner that you don't use the package anymore.

Ok, how large is "large"? And how many items are we talking about? And what platform are you on?

I don't use magit-todos anymore, so maybe the lag is no longer an issue. This is a good example of a big repo.

I started to download it to test, but seeing that it is at least 500 MB, I decided not to. Anyway, with a repository that large, then, yes, it can be expected to take a while.

FWIW, testing on the emacs.git repo (about 1.3 GB) using ripgrep on my system, it takes about 3 seconds to scan the repo's files (about 4,800) and and insert 2,300 items (on an SSD).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discussion invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

2 participants