[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] (icomplete-vertical-mode): Add support for affixations and,

From: Gregory Heytings
Subject: Re: [PATCH] (icomplete-vertical-mode): Add support for affixations and, annotations
Date: Tue, 01 Jun 2021 14:47:43 +0000

I didn't get a terrible amount of feedback, but I spent the week testing that branch, making small adjustments, and verifying that the default Icomplete and Fido behaviour are unchanged. So I've now pushed the icomplete-vertical-mode rework to master.

Okay, so I guess it's too late to send you feedback... I asked you to wait a few days more, and promised to send you feedback this week.

You can still send feedback, of course. Just because it's in master doesn't mean it's written in stone.

That's most often the case, but let's see whether this one is an exception.

I did wait a few days more to your request but I didn't think it made sense to wait many more as I need to move on to other developments that kind of depend on it. Anyway, I think you'll like the new mode, but let me know what you think.

I've now spent some time to test the feature and to look at your code, and here are a few comments:

1. Your code is not modular enough, for example, IMO it would be much better if the the annotation function and the "candidate number/total number of candidates" indication were (user-configurable) hooks, instead of hard-coding these operations in the code. Not everyone wants to see these numbers, not everyone wants to see annotations even if they are available, some might want to add annotations to the candidates list with their own chosen formatting, and so forth.

2. I don't understand why you used "icomplete-scroll" for the variable name, and why it is not a defcustom. IMO it should have been something more explicit like "icomplete-vertical-scroll-candidate-list". And with a hook, this variable would not be necessary.

3. As I feared, the code to scroll the candidates list does not work correctly alas. I've put lots of efforts to make "icomplete-vertical" work correctly in virtually every possible case, so I find this very regrettable. For example, with frame-height = 47, 10 candidates are displayed, yet the candidates list rotates on the 7th candidate (instead of the 11th one). With frame-height = 30, 6 candidates are displayed, and the candidates list rotates on the 6th candidate (instead of the 7th one). With frame-height = 22, 4 candidates are displayed, and the candidates list correctly rotates on the 5th candidate. With frame-height = 15, two candidates are displayed, yet the candidates list rotates on the 4th candidate (instead of the 3th one, and in this case you don't even see the current candidate anymore).

If I start working on this (again), I would likely change most of your code into something else, and it would take some time, because it's a difficult problem. Would you agree with this?

reply via email to

[Prev in Thread] Current Thread [Next in Thread]