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

fix: correctly print multi-line errors #270

Merged
merged 2 commits into from
Sep 5, 2024
Merged

fix: correctly print multi-line errors #270

merged 2 commits into from
Sep 5, 2024

Conversation

tigarmo
Copy link
Collaborator

@tigarmo tigarmo commented Sep 5, 2024

The bug manifested in a situation like this:

  • A long progress() message is printed, say "AAAAAAAA";
  • An error is emitter, and the result of str(error) is a multi-line string where the first line is shorter than the previously-emitted progress() message. Example: "BBBB\nCCCC";
  • The newline in the error message is not correctly handled by the printer, meaning that after "BBBB" is written to the terminal the remainder of the line is not cleared. So the end result looks like:

    BBBBAAAA
    CCCC

The "proper" fix for this would mean a big rework on the printer to properly support multi-line messages; for now, fix only the error case by "manually" splitting the error message and printing each line individually.

Fixes #263

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run tox?

@tigarmo
fix: correctly print multi-line errors
ad6a2ea 
The bug manifested in a situation like this:

- A long progress() message is printed, say "AAAAAAAA";
- An error is emitter, and the result of str(error) is a multi-line string
  where the first line is shorter than the previously-emitted progress()
  message. Example: "BBBB\nCCCC";
- The newline in the error message is not correctly handled by the printer,
  meaning that after "BBBB" is written to the terminal the remainder of the
  line is not cleared. So the end result looks like:
  > BBBBAAAA
  > CCCC

The "proper" fix for this would mean a big rework on the printer to properly
support multi-line messages; for now, fix only the error case by "manually"
splitting the error message and printing each line individually.

Fixes #263
@tigarmo tigarmo requested review from lengau and a team September 5, 2024 14:40
@lengau lengau requested a review from a team September 5, 2024 20:23
@lengau
Copy link
Contributor

lengau commented Sep 5, 2024

noice.gif

mattculler
mattculler approved these changes Sep 5, 2024
View reviewed changes
craft_cli/messages.py Outdated Show resolved Hide resolved
@tigarmo tigarmo merged commit 0ca6940 into main Sep 5, 2024
8 checks passed
@tigarmo tigarmo deleted the work/fix-multiline-errors branch September 5, 2024 20:41
github-merge-queue bot referenced this pull request in canonical/charmcraft Sep 6, 2024
@renovate
build(deps): update dependency craft-cli to v2.7.0 (main) (#1879)
72c1d43 
This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [craft-cli](https://redirect.github.com/canonical/craft-cli) |
`==2.6.0` -> `==2.7.0` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/craft-cli/2.7.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/craft-cli/2.7.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/craft-cli/2.6.0/2.7.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/craft-cli/2.6.0/2.7.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>canonical/craft-cli (craft-cli)</summary>

###
[`v2.7.0`](https://redirect.github.com/canonical/craft-cli/releases/tag/2.7.0)

[Compare
Source](https://redirect.github.com/canonical/craft-cli/compare/2.6.0...2.7.0)

##### What's Changed

- feat: add CraftCommandError by
[@&#8203;tigarmo](https://redirect.github.com/tigarmo) in
[https://github.com/canonical/craft-cli/pull/264](https://redirect.github.com/canonical/craft-cli/pull/264)
- ci: update renovate from starbase by
[@&#8203;lengau](https://redirect.github.com/lengau) in
[https://github.com/canonical/craft-cli/pull/268](https://redirect.github.com/canonical/craft-cli/pull/268)
- fix: correctly print multi-line errors by
[@&#8203;tigarmo](https://redirect.github.com/tigarmo) in
[https://github.com/canonical/craft-cli/pull/270](https://redirect.github.com/canonical/craft-cli/pull/270)
- docs: changelog entries for 2.7.0 by
[@&#8203;tigarmo](https://redirect.github.com/tigarmo) in
[https://github.com/canonical/craft-cli/pull/272](https://redirect.github.com/canonical/craft-cli/pull/272)

**Full Changelog**:
canonical/craft-cli@2.6.0...2.7.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "every weekend" in timezone Etc/UTC,
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/canonical/charmcraft).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC41OS4yIiwidXBkYXRlZEluVmVyIjoiMzguNTkuMiIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIl19-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
lengau referenced this pull request in canonical/charmcraft Sep 6, 2024
@renovate
build(deps): update dependency craft-cli to v2.7.0 (hotfix/3.2) (#1882)
c6d2a2e 
This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [craft-cli](https://redirect.github.com/canonical/craft-cli) |
`==2.6.0` -> `==2.7.0` |
[![age](https://developer.mend.io/api/mc/badges/age/pypi/craft-cli/2.7.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/pypi/craft-cli/2.7.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/pypi/craft-cli/2.6.0/2.7.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/pypi/craft-cli/2.6.0/2.7.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>canonical/craft-cli (craft-cli)</summary>

###
[`v2.7.0`](https://redirect.github.com/canonical/craft-cli/releases/tag/2.7.0)

[Compare
Source](https://redirect.github.com/canonical/craft-cli/compare/2.6.0...2.7.0)

##### What's Changed

- feat: add CraftCommandError by
[@&#8203;tigarmo](https://redirect.github.com/tigarmo) in
[https://github.com/canonical/craft-cli/pull/264](https://redirect.github.com/canonical/craft-cli/pull/264)
- ci: update renovate from starbase by
[@&#8203;lengau](https://redirect.github.com/lengau) in
[https://github.com/canonical/craft-cli/pull/268](https://redirect.github.com/canonical/craft-cli/pull/268)
- fix: correctly print multi-line errors by
[@&#8203;tigarmo](https://redirect.github.com/tigarmo) in
[https://github.com/canonical/craft-cli/pull/270](https://redirect.github.com/canonical/craft-cli/pull/270)
- docs: changelog entries for 2.7.0 by
[@&#8203;tigarmo](https://redirect.github.com/tigarmo) in
[https://github.com/canonical/craft-cli/pull/272](https://redirect.github.com/canonical/craft-cli/pull/272)

**Full Changelog**:
canonical/craft-cli@2.6.0...2.7.0

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "every weekend" in timezone Etc/UTC,
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/canonical/charmcraft).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC41OS4yIiwidXBkYXRlZEluVmVyIjoiMzguNTkuMiIsInRhcmdldEJyYW5jaCI6ImhvdGZpeC8zLjIiLCJsYWJlbHMiOlsiZGVwZW5kZW5jaWVzIl19-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lengau lengau lengau approved these changes

@mattculler mattculler mattculler approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Line overwrite causes corrupted error message
3 participants
@tigarmo @lengau @mattculler