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

Make the count string customizable #25

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

abo-abo
Copy link
Contributor

@abo-abo abo-abo commented Mar 19, 2015

  • ido-vertical-mode.el (ido-vertical-show-count): Update.
  • (ido-vertical-count-active): Remove.
  • (ido-vertical-completions): Modify `ido-decorations' instead.
  • (turn-on-ido-vertical): Use copy-sequence so that ido-vertical-decorations doesn't have to be modified.

* ido-vertical-mode.el (ido-vertical-show-count): Update.
(ido-vertical-count-active): Remove.
(ido-vertical-completions): Modify `ido-decorations' instead.
(turn-on-ido-vertical): Use `copy-sequence' so that
`ido-vertical-decorations' doesn't have to be modified.
@abo-abo
Copy link
Contributor Author

abo-abo commented Mar 19, 2015

Here's my current config that's enabled by the last change:

(use-package ido-vertical-mode
    :config
  (setf (nth 0 ido-vertical-decorations) "\n")
  (setf (nth 2 ido-vertical-decorations) "\n")
  (setf (nth 11 ido-vertical-decorations) "\n")
  (ido-vertical-mode 1))

* ido-vertical-mode.el (ido-vertical-completions): Simplify.
@gempesaw
Copy link
Collaborator

Hey, thanks for the PR!

I don't understand the summary here in the PR, or the commit messages, or how it relates to the config changes here. Can you explain it a bit, or split out the commits into more logical chunks? The messages are too terse for me.

Also, the CI tests are failing.

I initially had a suspicion that this should should be broken into two PRs (one for customizing count string, one for refactoring & simplifying the text-face handling), but after looking at the code again, I can't tell if they're all related or not...

@abo-abo
Copy link
Contributor Author

abo-abo commented Mar 21, 2015

The commits simplify the code and make it more customizable.
This is what I currently have by just customizing ido-vertical-mode:

ido-vertical-1

The main point is to re-use ido-vertical-decorations everywhere, so that when they are customized, everything is consistent. I recomment to use magit and ediff with "-w" parameter to look at the Elisp diffs: it makes them look a lot simpler.

@gempesaw
Copy link
Collaborator

Hmm, that looks like the after image, I guess, since the count string uses parens instead of brackets?And, the main point isn't about the count string at all then. Ah, with that in mind, I took another look and it makes a bit more sense. Oh, I see - the commits themselves seem to be entirely independent of each other & unrelated, but the summary view showed all the changes together, which made things less clear. Hm, separate PRs would've helped, but no big deal, it makes sense now.

I'll see if I have some time to figure out why the test is failing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants