Use internal property for directory suffix #256
Conversation
|
So you have both 'selectrum-candidate-display-suffix and 'selectrum--candidate-display-suffix now? Not that I am in any way familiar with the selectrum code base, but this looks like a typo to me. What about 'selectrum--internal-candidate-display-suffix and documenting the difference? Does this solve the problems with the directories in #255? |
|
As it's only used in one it's not hard to figure out what it used for but I can add a comment. I will also rename it that's a good idea!
Yes, it should |
|
@clemera Thank you, works perfectly! |
|
I got rid of the internal property again in #258. |
| ;; displaying slashes of directories in file | ||
| ;; completions. | ||
| 0 'selectrum--internal-candidate-display-suffix | ||
| candidate)) |
There was a problem hiding this comment.
Hmmm. This seems slightly unfortunate. It feels kind of similar to some of the things going on in Ivy/Counsel, where the public API isn't expressive enough to implement all the needed features, and so some of the Ivy functions have special cases to handle specific callers from Counsel. In an ideal world, we'd be able to support multiple suffixes natively in some way through the public API.
I don't know if that's actually achievable---maybe this is one of those cases where you just have to hack it, because the alternative is too inconvenient---but I thought I would call attention to it since I think we want to try to avoid this kind of thing suffusing the entire codebase, since then we'll end up in an Ivy/Counsel sort of situation.
There was a problem hiding this comment.
Oops, hehe, I see you got rid of it in #262. That's what I get for being so behind on email!
There was a problem hiding this comment.
I'm happy it could be removed because of the reasons you mentioned above, there where some refactoring issues after I switched but I hope these have all been sorted out now.
Using
selectrum-candidate-display-suffixoverrides any annotations or usages of that property from the calling side so I switched to use an internal property for the special purpose of displaying the directory slashes. See #255.