Add: (magit--imenu-goto-function) For imenu-default-goto-function

[alphapapa]

• lisp/magit-mode.el (magit--imenu-goto-function): New function.
  (magit-mode): Set locally in magit-mode as imenu-default-goto-function.

---

This new function automatically expands the jumped-to section, as well as its children and ancestors. Without this function, the section is not automatically expanded, which then requires the user to manually expand the ancestor sections and jump to it again.

Thanks for your work on Magit!

[alphapapa]

By the way, while I'm looking at this code: Would it make sense to move the imenu-related setq-locals into the magit-section-mode definition? That way they would be usable in all magit-section and derived buffers rather than only magit-mode and derived (which would be especially useful in other packages, like ement and others that use taxy-magit-section).

If this is agreeable, I'll submit a PR making the change. Thanks.

[tarsius]

Makes sense, will merge.

Please don't use cl-loop, I don't like it. There is other code that walks up the ancestry tree; do it like it's done there.

[tarsius]

By the way, while I'm looking at this code: Would it make sense to move the imenu-related setq-locals into the magit-section-mode definition? That way they would be usable in all magit-section and derived buffers rather than only magit-mode and derived (which would be especially useful in other packages, like ement and others that use taxy-magit-section).

If this is agreeable, I'll submit a PR making the change. Thanks.

Yes and no. Or rather the same could be said for nearly all the features implemented in magit-mode.el. The only thing that jumps out as be useless for non-magit modes, but even things like the magit-ACTION-thing stub commands are potentially useful elsewhere.

Moving stuff from magit-mode to magit-section also means it becomes harder to make breaking changes. So while I can see how doing that can be beneficial to others, for me it is just more work, for little to no gain.

But I should also note taht over the last few days this seems to have become the default response to every suggestion that means more work and/or locks me into having to stick to design decisions. I seem to be approaching burn out again.

So for now my answer is no. Please just copy-paste the implementation. You can ping me towards the end of the year again and ask me to reconsider. I haven't put done the foot down enough for to long and might be overcompensating now, but right now it is a necessary defense mechanism (at least I am aware of it). I might change my opinion once the constant stream of "just one more thing" has stopped and I had a change to work on the things I want to work on for a while.

[alphapapa]

Makes sense, will merge.

Please don't use cl-loop, I don't like it. There is other code that walks up the ancestry tree; do it like it's done there.

Ok, is this what you had in mind? 8066048

[alphapapa]

So for now my answer is no. Please just copy-paste the implementation.

Ok. Thanks.

[tarsius]

Ok, is this what you had in mind? 8066048

Yes, but we don't actually need two variables. We can also avoid expanding a section if it is already expanded.

I don't think it is appropriate to extend the target section itself. One might, for example, want to jump to a modified file in order to stage it, without wanting to look at its diff first.

I've also adjusted the commit message to be in line with our conventions.

[tarsius]

Before I merge; does this still do what you had in mind (modulo the expansion of the target section)?

[alphapapa]

Yes, thanks.

[alphapapa]

I don't think it is appropriate to extend the target section itself. One might, for example, want to jump to a modified file in order to stage it, without wanting to look at its diff first.

Makes sense. Thanks.

I've also adjusted the commit message to be in line with our conventions.

Thanks. Is this convention documented? I looked in various places in the repo's tree and on the GitHub pages, like https://github.com/magit/magit/wiki/Pull-request-guidelines, but couldn't find an example.

[tarsius]

Is this convention documented?

No and if I wrote one, I fear it would either be either annoyingly vague or annoyingly pedantic. Maybe I could put in the work and produce something that is at least also funny, but I fear that day will never come.

The thing is, we basically use "write a good commit message, as described in 95% of blog posts defining 'good commit message'", modulo some "not all commits are equal" and "if there is nothing to be gained don't make me jump through hoops". Then we throw in our own idiosyncrasies, such as using elisp-docstring-style symbol quoting and "we begin the summary line with 'the' modified symbol, except when we don't".

My hope is, that contributors do what I do, when I contribute to a project foreign to me; quickly look at half a dozen commits by the maintainer, and then try to do the same.

We are pretty lax about it really. For example, Kyle and I use different styles of referring to older commits, and contributors use other style, usually (abc123), and that is perfectly fine.

Usually I just make the necessary little adjustments myself right before merging. I sometimes mention that when closing the pull-request, but not to say "you better get this right next time" but as a log in case I somehow introduced a mistake while touching things up, to take blame from the contributor.

I am a bit surprised when contributors use "conventional commit" (or similar) or gnu-style formatting. These can be useful (well, at least the former), but only if done consistently. A commit following such a style, among a sea of commits that don't, is just very odd.

So in summary: Summarize on the first line, describe why changes are needed (provided doing that is actually useful), look at a handful of commits to hopefully pick up on our idiosyncrasies, use common sense and if you make a "mistake", I'll fix it.

[alphapapa]

My hope is, that contributors do what I do, when I contribute to a project foreign to me; quickly look at half a dozen commits by the maintainer, and then try to do the same.

When I can't find docs, I fall back to that, but in this case I failed to (quickly) discern a consistent pattern, so went with my usual style. What I was most concerned about was the first-line style, so it might be worth mentioning that pattern, at least.

Thanks as always for your work, Jonas. I can't imagine doing the work I do without having Magit by my side.

[tarsius]

I've added the "maintainer's bookmark" label for now. 😉