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

k8s-state: update-last-error: Fix non-fatal syntax error #155

Merged

Conversation

jinnovation
Copy link
Collaborator

undercover fails to parse this particular line with the following
message:

UNDERCOVER: Error while loading /Users/jjin/dev/kubernetes-el/kubernetes-state.el for coverage:
UNDERCOVER: Invalid read syntax: "."
UNDERCOVER: The problem may be due to edebug failing to parse the file.
UNDERCOVER: You can try to narrow down the problem using the following steps:
UNDERCOVER: 1. Open "/Users/jjin/dev/kubernetes-el/kubernetes-state.el" in an Emacs buffer;
UNDERCOVER: 2. Run M-: ‘(require 'edebug)’;
UNDERCOVER: 3. Run M-x ‘edebug-all-defs’;
UNDERCOVER: 4. Run M-x ‘toggle-debug-on-error’.
UNDERCOVER: 5. Run M-x ‘eval-buffer’;
UNDERCOVER: 6. In the *Backtrace* buffer, find a numeric position,
UNDERCOVER:    then M-x ‘goto-char’ to it.

We can reproduce this w/ edebug-defun on the definition.

As it turns out, substitution with , in a backquoted list runs "just
fine" if the comma is not attached to the element to expand, as we can
reproduce here:

(let ((foo t))
  `(, foo))

However, we fix up the definition for full correctness and so that
undercover doesn't complain.

`undercover` fails to parse this particular line with the following
message:
```
UNDERCOVER: Error while loading /Users/jjin/dev/kubernetes-el/kubernetes-state.el for coverage:
UNDERCOVER: Invalid read syntax: "."
UNDERCOVER: The problem may be due to edebug failing to parse the file.
UNDERCOVER: You can try to narrow down the problem using the following steps:
UNDERCOVER: 1. Open "/Users/jjin/dev/kubernetes-el/kubernetes-state.el" in an Emacs buffer;
UNDERCOVER: 2. Run M-: ‘(require 'edebug)’;
UNDERCOVER: 3. Run M-x ‘edebug-all-defs’;
UNDERCOVER: 4. Run M-x ‘toggle-debug-on-error’.
UNDERCOVER: 5. Run M-x ‘eval-buffer’;
UNDERCOVER: 6. In the *Backtrace* buffer, find a numeric position,
UNDERCOVER:    then M-x ‘goto-char’ to it.
```

We can reproduce this w/ `edebug-defun` on the definition.

As it turns out, substitution with `,` in a backquoted list runs "just
fine" if the comma is not attached to the element to expand, as we can
reproduce here:
```emacs-lisp
(let ((foo t))
  `(, foo))
```

However, we fix up the definition for full correctness and so that
undercover doesn't complain.
@coveralls
Copy link

coveralls commented Jun 4, 2021

Coverage Status

Coverage increased (+9.2%) to 45.671% when pulling db98481 on jinnovation:non-fatal-backquote-syntax into b887d96 on chrisbarrett:master.

@jinnovation
Copy link
Collaborator Author

Coverage increased (+9.2%) to 45.603%

Nice.

@noorul
Copy link
Collaborator

noorul commented Jun 4, 2021

@jinnovation Will, you be interested in becoming co-maintainer of this project?

@noorul noorul merged commit 93d7b4d into kubernetes-el:master Jun 4, 2021
@jinnovation
Copy link
Collaborator Author

@jinnovation Will, you be interested in becoming co-maintainer of this project?

I'd be happy to. That said, I can't and won't commit to always being as active as I have been lately.

Go ahead and add me in.

@noorul
Copy link
Collaborator

noorul commented Jun 4, 2021

@chrisbarrett Can you please add @jinnovation to contributors list?

@jinnovation jinnovation deleted the non-fatal-backquote-syntax branch June 4, 2021 10:04
@jinnovation
Copy link
Collaborator Author

@chrisbarrett Can you please add @jinnovation to contributors list?

@chrisbarrett: Bumping on this. Think you could help out here?

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

3 participants