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

Feature: Customizable buttons in zk-desktop, fix for overlay bug #76

Open
wants to merge 45 commits into
base: main
Choose a base branch
from

Conversation

boyechko
Copy link
Contributor

This started as a frustration with the persistent <-- ID NOT FOUND overlays which kept increasing after each new desktop entry, so I'd end up with lines of <-- ID NOT FOUND <-- ID NOT FOUND <-- ID NOT FOUND <-- .... One thing led to another, and the result is I believe a more customizable and flexible Zk-Desktop.

Main improvements:

  • Overlays and text properties get cleared when remaking buttons
  • Format of lines and entries can be fully customized via zk-desktop-entry-format, zk-desktop-entry-prefix, and zk-desktop-entry-suffix
  • zk-index--current-id-list works on region, if active
  • Sending to desktop from zk-index respects zk-desktop-entry-* variables rather than just copying the zk-index region
  • zk-desktop-mode is a properly toggling minor mode
  • zk-desktop--make-button makes for much faster zk-desktop-send-to-desktop

I am including the entire commit history that you can see the incremental changes, but many/all can be squashed. I know you don't have time to review new features, I'm just parking this here if that changes at any point. Given the scale of the changes to zk-desktop.el, though, I'd appreciate some acknowledgment in the file.

Provides more feedback to the user if they call `zk-desktop-make-buttons`
directly and from the wrong buffer.
Preserves horizontal space by eliminating one level of indentation.
Makes `zk-desktop-make-buttons` less imposing and self-documents.
The function is easier to read and with fewer searches and replacements, there
is less potential for errors to crop up.
This makes the code more modular and easier to maintain.
The name, along with the updated docstring, clarifies what this actually means.
This clarifies what are the actual bounds for the button being made.
There were two issue:

1. With `zk-desktop-invisible-ids` set to T, we make the IDs invisible with text
    properties and overlays; setting `..-invisible-ids` to NIL and re-doing
    `zk-desktop-make-buttons` not only does not reveal invisible IDs, but makes
    the titles invisible too.
2. The overlay " <- ID NOT FOUND" stays in the buffer, so that repeated calls to
   `zk-desktop-make-buttons` results in more and more of these overlays being
   added to each line with a missing ID.
Working to make `zk-desktop-send-to-desktop` more modular and simpler to
maintain.
Despite `completing-read`'s docstring, this is a good case where INITIAL-INPUT
ought to be used, since zk-desktop-basename is by no means a default value; it's
just the beginning of any desktop file name.
Either `zk-desktop-current` is already a live buffer or `zk-desktop-select`
creates one, there should be no case where we'd need to `generate-new-buffer`.
Especially since it expects a name of a new buffer rather than whatever `buffer`
variable ends up being.
Adding the 'type property to overlays makes it possible to later selectively
remove "our" overlays, which is what `zk-desktop--clear' does.
While `zk-desktop-prefix` is a static value, being able to pass a string for
every item sent to the desktop allows for things like time-stamps or even brief
comments.
This allows the user an option to treat Zk-Desktop file as a regular text
file (in other words, without buttons, read-only text properties, or overlays).
Someone who uses org-mode could, for example, just set it up that the Zk-Desktop
files are annotated org-mode documents, with each item as a heading or a list
item by setting `zk-desktop-prefix` to `"** "` or `"- "`, respectively.
This is primarily useful for `zk-desktop--gather-items`, but might come in handy
elsewhere too.
This requires extending `zk-index--current-id-list` to return IDs within the
active region, but makes it possible to produce consistently-formatted
Zk-Desktop file rather than relying on the customizable format of Zk-Index.
Renaming these to `zk-desktop-entry-prefix` and `zk-desktop-entry-format`
distinguishes them from actual Emacs buttons that may or may not be created
according to `zk-desktop-make-buttons`.
The suffix argument to `zk-desktop-send-to-desktop` allows adding arbitrary
text, such as the current time, for example, to each individual entry. In the
future, the `-suffix` and `-prefix` variables can accept functions as well as
strings to make that easier.
This allows customizing how missing IDs are handled.
Previously, `zk-desktop--make-button` relied on match-data from
`zk-desktop-make-buttons`, which meant that it couldn't be called from other
functions. While rewriting, also made a series of improvements:

- `zk-desktop-make-buttons` only gathers list of IDs if
  `zk-desktop-mark-missing` is non-nil.
- When zk-desktop-mark-missing is non-nil, but not a string, set the button face
  to `zk-desktop-missing-button`.
- Add `button-data` text button property that contains the ID and TITLE for the
  zk-desktop entry.
If the desktop file is long, this is quite a significant performance boost for
each call to `zk-desktop-send-to-desktop`.
@localauthor
Copy link
Owner

Wow, thanks for this! I'll try to give it a look soon.

boyechko added a commit to boyechko/octavo that referenced this pull request Sep 16, 2023
(concat
zk-desktop-basename
".*"))
nil nil zk-desktop-basename nil))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the doc-string for completing-read, regarding use of INITIAL-INPUT:
"This feature is deprecated--it is best to pass nil
for INITIAL-INPUT and supply the default value DEF instead. The
user can yank the default value into the minibuffer easily using
M-n."

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I realize that about completing-read, but sometimes it just doesn't make sense. zk-desktop-basename is not the default value; it is a common portion of zk-desktop names that the user then adds to. INITIAL-INPUT seems like the most natural place to put that in, though perhaps we could add something to the prompt text instead? Like "Select or Create ZK-Desktop (following [value of zk-desktop-basename]): but less awkward...

@localauthor
Copy link
Owner

localauthor commented Sep 16, 2023

Some notes after a quick test:

  • make-buttons no longer converts plain zk links to desktop buttons, as it did previously; any reason for this change?

  • when zk-desktop-send-to-desktop is called from zk-index, either on a file at point or on a selection, make-buttons doesn't make buttons. [note: this is when zk-desktop-add-pos is set to 'at-point]

  • zk-desktop-send-to-desktop doesn't work when called with embark (something missing in gather-items?)

  • when an ID is not found, the line is still made into a button, and then zk-desktop-mark-missing is added after; seems like it shouldn't be made into a button at all

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.

None yet

2 participants