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

:pad-keys appears to be broken since ae5aba367c3ff83d2457e725781290b6810b4e1a #269

Closed
alphapapa opened this issue Dec 16, 2023 · 5 comments

Comments

@alphapapa
Copy link

Hi Jonas,

Since I upgraded Transient to 0.5, my makem.el dispatcher seems to be broken. When I run M-x makem RET I get this backtrace:

Debugger entered--Lisp error: (wrong-type-argument number-or-marker-p t)
  transient--exit-and-debug(error (wrong-type-argument number-or-marker-p t))
  truncate-string-to-width("-d" t nil 32)
  #f(compiled-function (obj) "Format OBJ's `key' for display and return the result." #<bytecode 0x113017db69503154>)(#<transient-switch transient-switch-2088e4c>)
  apply(#f(compiled-function (obj) "Format OBJ's `key' for display and return the result." #<bytecode 0x113017db69503154>) #<transient-switch transient-switch-2088e4c>)
  #f(compiled-function (&rest args) #<bytecode -0x13ac39a4dd84872d>)()
  #f(compiled-function (cl--cnm obj) #<bytecode -0xd91c63b8feb4dba>)(#f(compiled-function (&rest args) #<bytecode -0x13ac39a4dd84872d>) #<transient-switch transient-switch-2088e4c>)
  apply(#f(compiled-function (cl--cnm obj) #<bytecode -0xd91c63b8feb4dba>) #f(compiled-function (&rest args) #<bytecode -0x13ac39a4dd84872d>) #<transient-switch transient-switch-2088e4c>)
  #f(compiled-function (obj) "Add `transient-inapt-suffix' face if suffix is inapt." #<bytecode -0x199a03a64abc9356>)(#<transient-switch transient-switch-2088e4c>)
  apply(#f(compiled-function (obj) "Add `transient-inapt-suffix' face if suffix is inapt." #<bytecode -0x199a03a64abc9356>) #<transient-switch transient-switch-2088e4c>)
  #f(compiled-function (&rest args) #<bytecode 0x1b2de3a9dc2f18f7>)(#<transient-switch transient-switch-2088e4c>)
  #f(compiled-function (cl--cnm obj) #<bytecode -0x5155fe0d273d0a3>)(#f(compiled-function (&rest args) #<bytecode 0x1b2de3a9dc2f18f7>) #<transient-switch transient-switch-2088e4c>)
  apply(#f(compiled-function (cl--cnm obj) #<bytecode -0x5155fe0d273d0a3>) #f(compiled-function (&rest args) #<bytecode 0x1b2de3a9dc2f18f7>) #<transient-switch transient-switch-2088e4c>)
  #f(compiled-function (obj) "Handle `transient-highlight-mismatched-keys'." #<bytecode 0x1908b0d646bf00f5>)(#<transient-switch transient-switch-2088e4c>)
  apply(#f(compiled-function (obj) "Handle `transient-highlight-mismatched-keys'." #<bytecode 0x1908b0d646bf00f5>) #<transient-switch transient-switch-2088e4c> nil)
  transient-format-key(#<transient-switch transient-switch-2088e4c>)
  #f(compiled-function (obj) "Return a string generated using OBJ's `format'.\n%k is formatted using `transient-format-key'.\n%d is formatted using `transient-format-description'.\n%v is formatted using `transient-format-value'." #<bytecode -0xa94e9c8a6ddb31c>)(#<transient-switch transient-switch-2088e4c>)
  apply(#f(compiled-function (obj) "Return a string generated using OBJ's `format'.\n%k is formatted using `transient-format-key'.\n%d is formatted using `transient-format-description'.\n%v is formatted using `transient-format-value'." #<bytecode -0xa94e9c8a6ddb31c>) #<transient-switch transient-switch-2088e4c>)
  #f(compiled-function (&rest args) #<bytecode 0x20db16445fea1d7>)(#<transient-switch transient-switch-2088e4c>)
  #f(compiled-function (cl--cnm obj) #<bytecode 0x92b629b2a7c6a8f>)(#f(compiled-function (&rest args) #<bytecode 0x20db16445fea1d7>) #<transient-switch transient-switch-2088e4c>)
  apply(#f(compiled-function (cl--cnm obj) #<bytecode 0x92b629b2a7c6a8f>) #f(compiled-function (&rest args) #<bytecode 0x20db16445fea1d7>) #<transient-switch transient-switch-2088e4c>)
  #f(compiled-function (obj) "Add additional formatting if appropriate.\nWhen reading user input for this infix, then highlight it.\nWhen edit-mode is enabled, then prepend the level information.\nWhen `transient-enable-popup-navigation' is non-nil then format\nas a button." #<bytecode -0x1df835336cf646e>)(#<transient-switch transient-switch-2088e4c>)
  apply(#f(compiled-function (obj) "Add additional formatting if appropriate.\nWhen reading user input for this infix, then highlight it.\nWhen edit-mode is enabled, then prepend the level information.\nWhen `transient-enable-popup-navigation' is non-nil then format\nas a button." #<bytecode -0x1df835336cf646e>) #<transient-switch transient-switch-2088e4c> nil)
  transient-format(#<transient-switch transient-switch-2088e4c>)
  mapcar(transient-format (#<transient-switch transient-switch-2088e4c> #<transient-switch transient-switch-2088e9c> #<transient-switch transient-switch-1f03d28> #<transient-option transient-option-1f03d78>))
  #f(compiled-function (column) #<bytecode -0x7df8ba5a41e1b11>)(#<transient-column transient-column-2088e2c>)
  mapcar(#f(compiled-function (column) #<bytecode -0x7df8ba5a41e1b11>) (#<transient-column transient-column-2088e2c> #<transient-column transient-column-2088eec>))
  #f(compiled-function (group) #<bytecode -0xf7c632005a17f63>)(#<transient-columns transient-columns-2088e0c>)
  apply(#f(compiled-function (group) #<bytecode -0xf7c632005a17f63>) #<transient-columns transient-columns-2088e0c>)
  #f(compiled-function (&rest args) #<bytecode 0xf004b0aa7981c77>)(#<transient-columns transient-columns-2088e0c>)
  #f(compiled-function (cl--cnm group) #<bytecode 0x1d24d625a00a224c>)(#f(compiled-function (&rest args) #<bytecode 0xf004b0aa7981c77>) #<transient-columns transient-columns-2088e0c>)
  apply(#f(compiled-function (cl--cnm group) #<bytecode 0x1d24d625a00a224c>) #f(compiled-function (&rest args) #<bytecode 0xf004b0aa7981c77>) #<transient-columns transient-columns-2088e0c>)
  #f(compiled-function (group) "Insert GROUP's description, if any." #<bytecode -0x195b6e5284026c29>)(#<transient-columns transient-columns-2088e0c>)
  apply(#f(compiled-function (group) "Insert GROUP's description, if any." #<bytecode -0x195b6e5284026c29>) #<transient-columns transient-columns-2088e0c> nil)
  transient--insert-group(#<transient-columns transient-columns-2088e0c>)
  transient--insert-groups()
  transient--show()
  transient--redisplay()
  transient-setup(makem)
  makem()
  funcall-interactively(makem)
  command-execute(makem record)
  execute-extended-command(nil "makem" nil)
  funcall-interactively(execute-extended-command nil "makem" nil)
  command-execute(execute-extended-command)

It appears to me that this was broken in ae5aba3. It seems that the pad-keys value of t is being passed to truncate-string-to-width, rather than the intended, computed value.

Thanks for your work.

@tarsius
Copy link
Member

tarsius commented Dec 16, 2023

Thanks for the report!

So while we are finding issues in each others code, I would like to remind you of this:

In toplevel form:
lib/hyperdrive/hyperdrive-mirror.el:92:2: Warning: in defcustom for ‘hyperdrive-mirror-level-indent’: fails to specify containing group
lib/hyperdrive/hyperdrive-mirror.el:92:2: Warning: in defcustom for ‘hyperdrive-mirror-item-indent’: fails to specify containing group

And while we are at it:

In toplevel form:
lib/magit-todos/magit-todos.el:76:11: Warning: ‘buffer-substring’ is an obsolete generalized variable.

In end of data:
lib/magit-todos/magit-todos.el: Warning: the function ‘reb-target-binding’ is not known to be defined.

(I'm trying to get all the packages I have installed to zero warnings by the end of the year. 😁 )

@tarsius
Copy link
Member

tarsius commented Dec 16, 2023

Oh, and I almost forgot; why isn't makem on Melpa? Sure makem.sh is supposed to be imported into the project, but what about makem.el?

@alphapapa
Copy link
Author

alphapapa commented Dec 17, 2023

Thanks for the report!

So while we are finding issues in each others code, I would like to remind you of this:

In toplevel form:
lib/hyperdrive/hyperdrive-mirror.el:92:2: Warning: in defcustom for ‘hyperdrive-mirror-level-indent’: fails to specify containing group
lib/hyperdrive/hyperdrive-mirror.el:92:2: Warning: in defcustom for ‘hyperdrive-mirror-item-indent’: fails to specify containing group

That could be solved by putting a defgroup in hyperdrive-mirror.el before the taxy-magit-section-define-column-definer form.

And while we are at it:

In toplevel form:
lib/magit-todos/magit-todos.el:76:11: Warning: ‘buffer-substring’ is an obsolete generalized variable.

I won't stop using that, because it shouldn't have been marked obsolete in the first place. Of all the forms that shouldn't have been obsoleted, that one is certainly near the top of the list, because of how convenient it is, and how awkward the equivalent code is. When I complained about the mass-obsoleting on the mailing list, I was told to individually justify each one I wanted to not be obsoleted (but it was okay for one developer to en masse mark-as-obsolete forms used in the wild, without discussing it first--and then he virtually disappeared). Who has time for that, but I've at least taken care of one so far: https://debbugs.gnu.org/cgi/bugreport.cgi?bug=65555 So I'll file a bug about that when I get around to it. In the meantime, Eli said that obsolescence warnings are the invitation to discussion, not a promise to remove the feature, so the more warnings about that, the better, I guess.

In end of data:
lib/magit-todos/magit-todos.el: Warning: the function ‘reb-target-binding’ is not known to be defined.

That symbol doesn't even appear in magit-todos. It's an issue in the pcre2el library. See joddie/pcre2el#51 and joddie/pcre2el#54

(I'm trying to get all the packages I have installed to zero warnings by the end of the year. 😁 )

Good luck with that. It's an eternal game of whack-a-mole. Every new version of Emacs makes new warnings, and solving them on the latest release causes other warnings on older ones. So it's practically impossible to have a package lint and install cleanly on multiple versions.

Oh, and I almost forgot; why isn't makem on Melpa? Sure makem.sh is supposed to be imported into the project, but what about makem.el?

Because I haven't submitted it yet. And I'm not sure if I want to put it on MELPA or GNU ELPA.

@alphapapa
Copy link
Author

@tarsius Apologies for being curt in my response. I appreciate your bringing issues to my attention. Those in particular just happened to be ones that I've been dealing with for a while.

Regarding makem: Do you have any opinion about whether I should submit it to MELPA or GNU ELPA?

@tarsius
Copy link
Member

tarsius commented Dec 18, 2023

Apologies for being curt in my response. I appreciate your bringing issues to my attention. Those in particular just happened to be ones that I've been dealing with for a while.

No worries, I didn't take it in a negative way.

[buffer-substring]

Thanks for bringing back (setf (buffer-local-value ...)...). I've silenced the warning about buffer-substring in borg and will patiently wait to see how that plays out.

[pcre2el]

I've submitted joddie/pcre2el#56 for that. Maybe one day I'll also port the advices to "new"advice.

Good luck with that. It's an eternal game of whack-a-mole.

Sure is. But I am winning! Currently I am getting warnings for two out of 107 installed packages.

(Often I have to open pull-requests to get those things addressed, and sometimes I even have to maintain local patches (but with borg that's not much of an issue).)

Regarding makem: Do you have any opinion about whether I should submit it to MELPA or GNU ELPA?

After writing a sentence suggesting one or the other, and then erasing that again, two or three times... no I don't think, I have a strongly held opinion on that.

zhscn pushed a commit to zhscn/nongnu that referenced this issue Dec 26, 2023
The options defined with `taxy-magit-section-define-column-definer'
will be put into that group.

Suggested-by: Adam Porter <adam@alphapapa.net>
magit/transient#269 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants