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.

Sign up for GitHub

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

Jump to bottom

format.kak - missing error message if external program failed #3254

Closed
mzanibelli opened this issue Dec 11, 2019 · 4 comments · Fixed by #4582
Closed

format.kak - missing error message if external program failed #3254

mzanibelli opened this issue Dec 11, 2019 · 4 comments · Fixed by #4582

Comments

@mzanibelli
Copy link

mzanibelli commented Dec 11, 2019 •
edited

Version: Kakoune v2019.12.10

Steps

External program used as formatter fails (returns non-zero code)

Outcome

No error message is displayed

Expected

The error message formatter returned an error <code> is displayed
@Screwtapello
Copy link
Contributor

Steps to reproduce:

  • Start Kakoune without a file, so it defaults to the scratch buffer which doesn't have any autoformatting config by default

  • Set a formatter that always fails:

      :set window formatcmd /bin/false

  • Run the formatter:

      :format

Actual outcome:

  • The *debug* buffer gets new messages:

      error running command 'eval -client client0 %{ fail formatter returned an error 1 }
  ': formatter returned an error 1

Expected outcome:

  • The error should be displayed to the user.

The strange error message comes from the way formatting.kak returns the error message to Kakoune. It runs the formatter asynchronously, then to report the result does the moral equivalent of:

echo eval -client $kak_client %{ fail formatter returned an error $? } |
    kak -p $kak_session

If I run these commands directly inside Kakoune, they both appear in the status line:

echo blort
fail blort

If I run these commands directly inside Kakoune, they both appear in the status line:

eval -client client0 echo blort
eval -client client0 fail blort

If I run these commands from the shell, the echo appears in the status line, but the fail only appears in *debug*:

echo eval -client client0 %{ echo blort } | kak -p failsess
echo eval -client client0 %{ fail blort } | kak -p failsess

If I run these commands from the shell, the echo does not appear, but the fail appears in *debug*:

echo eval %{ echo blort } | kak -p failsess
echo eval %{ fail blort } | kak -p failsess

It seems like fail always throws an error in the current client, even when eval -client is trying to execute it in the context of a different client.

This change was introduced with 5b1f925 which switched from echo -markup {Error} to a real fail. I think using a real fail makes sense, but apparently it doesn't work the way we expected.

@lenormf
Copy link
Contributor

lenormf commented Dec 13, 2019

The spell module suffers from the same bug. Reproducer: :spell xx.

@mzanibelli
Copy link
Author

@Screwtapello thanks for such a detailed bug report! I was in a hurry...

Indeed, throw failure coming from fail seems to be handled differently depending on the current execution context. That's easily fixed by editing RC scripts with simple echos while the bug is still present.

Cheers!

@lenormf lenormf mentioned this issue Feb 1, 2020
Closed
Screwtapello added a commit to Screwtapello/kakoune that referenced this issue Feb 5, 2020
@Screwtapello
rc lint: Add a lint-selections command to lint part of a buffer.
a58bafa 
Fixes mawww#3336.

Addresses parts of mawww#3254, mawww#3155, mawww#2302.

Changes include:

- New `lint-selections` command that only lints the current selections.
- New `lint-buffer` command that always lints the whole buffer.
- `lint` alias for `lint-buffer`, for backwards compatibility.
- Errors and warnings are now shown in the Error and Information faces,
  not hard-coded red and yellow.
- Error and warning flags now use "!" and "?" symbols respectively,
  instead of a unicode block, so they can still be distinguished
  in a monochrome colour-scheme or by colour-blind users.
- An error flag on a given line always takes precedence over a warning.
- All messages for the same line are collected into a multi-line message.
- We no longer escape tildes in messages, since that change added
  in commit ae339dc (2016) when we used `%~~` to quote messages.
  We stopped using `%~~` to quote messages in commit 1a2eecd (2018).
- Anything the linter writes to stderr is logged to the *debug* buffer,
  not lost.
- If the linter writes to stderr, an error is shown to the user instead
  of the usual error/warning count.
- Hidden `lint_errors` option is now a line-specs, not range-specs,
  since we didn't use the other three coordinates, just the line number.
Screwtapello added a commit to Screwtapello/kakoune that referenced this issue Feb 7, 2020
@Screwtapello
rc lint: Add a lint-selections command to lint part of a buffer.
e94668e 
Fixes mawww#3336.

Addresses parts of mawww#3254, mawww#3155, mawww#2302.

Changes include:

- New `lint-selections` command that only lints the current selections,
  and allows a custom lint command.
- New `lint-buffer` command that always lints the whole buffer with
  the linter specified in the lintcmd option.
- `lint` alias for `lint-buffer`, for backwards compatibility.
- Errors and warnings are now shown in the Error and Information faces,
  not hard-coded red and yellow.
- Error and warning flags now use "!" and "?" symbols respectively,
  instead of a unicode block, so they can still be distinguished
  in a monochrome colour-scheme or by colour-blind users.
- An error flag on a given line always takes precedence over a warning.
- All messages for the same line are collected into a multi-line message.
- We no longer escape tildes in messages, since that change added
  in commit ae339dc (2016) when we used `%~~` to quote messages.
  We stopped using `%~~` to quote messages in commit 1a2eecd (2018).
- Anything the linter writes to stderr is logged to the *debug* buffer,
  not lost.
- If the linter writes to stderr, an error is shown to the user instead
  of the usual error/warning count.
- The `lint_errors` hidden option is replaced by lint_messages,
  because it contains warnings as well as errors.
- `lint-next-error` renamed to `lint-next-message,
  and `lint-previous-error` renamed to `lint-previous-message`
  for the same reason.
    - New `lint-next-error` and `lint-previous-error` aliases,
      for backwards compatibility.
- `lint-next-message` and `lint-previous-message` show the message
  they jump to.
- Where `lint_errors` was a range-specs option, `lint_messages` is a
  line-specs option to keep things simpler. This means lint-next-message
  and lint-previous-message no longer jump to a specific column.
Screwtapello added a commit to Screwtapello/kakoune that referenced this issue Feb 7, 2020
@Screwtapello
rc lint: Add a lint-selections command to lint part of a buffer.
f34a01d 
Fixes mawww#3336.

Addresses parts of mawww#3254, mawww#3155, mawww#2302.

Changes include:

- New `lint-selections` command that only lints the current selections,
  and allows a custom lint command.
- New `lint-buffer` command that always lints the whole buffer with
  the linter specified in the lintcmd option.
- `lint` alias for `lint-buffer`, for backwards compatibility.
- Errors and warnings are now shown in the Error and Information faces,
  not hard-coded red and yellow.
- Error and warning flags now use "!" and "?" symbols respectively,
  instead of a unicode block, so they can still be distinguished
  in a monochrome colour-scheme or by colour-blind users.
- An error flag on a given line always takes precedence over a warning.
- All messages for the same line are collected into a multi-line message.
- We no longer escape tildes in messages, since that change added
  in commit ae339dc (2016) when we used `%~~` to quote messages.
  We stopped using `%~~` to quote messages in commit 1a2eecd (2018).
- Anything the linter writes to stderr is logged to the *debug* buffer,
  not lost.
- If the linter writes to stderr, an error is shown to the user instead
  of the usual error/warning count.
- The `lint_errors` hidden option is replaced by lint_messages,
  because it contains warnings as well as errors.
- `lint-next-error` renamed to `lint-next-message,
  and `lint-previous-error` renamed to `lint-previous-message`
  for the same reason.
    - New `lint-next-error` and `lint-previous-error` aliases,
      for backwards compatibility.
- `lint-next-message` and `lint-previous-message` show the message
  they jump to.
- Where `lint_errors` was a range-specs option, `lint_messages` is a
  line-specs option to keep things simpler. This means lint-next-message
  and lint-previous-message no longer jump to a specific column.
@lenormf lenormf mentioned this issue Apr 26, 2020
Closed
@Screwtapello
Copy link
Contributor

I think this would be fixed by #2292

krobelus added a commit to krobelus/kakoune that referenced this issue Apr 9, 2022
@krobelus
rc format: restore in-client error when formatcmd fails
7d2d372 
Commit 5b1f925 (rc: Use the standard `fail` command to report errors,
2019-11-14) replaced uses of "echo -markup {Error}" with "fail".
This made format.kak do

	echo "eval -client $kak_client %{ fail }" | kak -p $kak_session

Unfortunately "fail" fails in the client spawned by "kak -p" and not
in $kak_client where the user would see the message. Correct this.

While at it, clarify the error message, so users immediately know
that the number is the exit code.

Fixes mawww#3254
@krobelus krobelus mentioned this issue Apr 9, 2022
Merged
@mawww mawww closed this as completed in a0477b1 Apr 11, 2022
TeddyDD pushed a commit to TeddyDD/kakoune that referenced this issue Aug 19, 2022
@krobelus @TeddyDD
rc format: restore in-client error when formatcmd fails
8e19ac8 
Commit 5b1f925 (rc: Use the standard `fail` command to report errors,
2019-11-14) replaced uses of "echo -markup {Error}" with "fail".
This made format-buffer do

	echo "eval -client $kak_client %{ fail }" | kak -p $kak_session

Unfortunately "fail" fails in the client spawned by "kak -p" and not
in $kak_client where the user would see the message. Correct this.

While at it, clarify the error message, so users immediately know
that the number is the exit code.

Fixes mawww#3254
Gskartwii pushed a commit to Gskartwii/kakoune that referenced this issue Nov 11, 2022
@krobelus @Gskartwii
rc format: restore in-client error when formatcmd fails
d9a0d9d 
Commit 5b1f925 (rc: Use the standard `fail` command to report errors,
2019-11-14) replaced uses of "echo -markup {Error}" with "fail".
This made format-buffer do

	echo "eval -client $kak_client %{ fail }" | kak -p $kak_session

Unfortunately "fail" fails in the client spawned by "kak -p" and not
in $kak_client where the user would see the message. Correct this.

While at it, clarify the error message, so users immediately know
that the number is the exit code.

Fixes mawww#3254
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

rc format: restore in-client error when formatcmd fails krobelus/kakoune
3 participants
@lenormf @Screwtapello @mzanibelli