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

Built-in diff editor doesn't redraw screen correctly in the presence of tabs #4001

Closed
fschoenm opened this issue Jul 1, 2024 · 7 comments · Fixed by #4639
Closed

Built-in diff editor doesn't redraw screen correctly in the presence of tabs #4001

fschoenm opened this issue Jul 1, 2024 · 7 comments · Fixed by #4639
Labels
🐛bug Something isn't working scm-record Issues relating to the scm-record library, used as the default interactive diff/merge editor.

Comments

@fschoenm
Copy link

fschoenm commented Jul 1, 2024

Description

In some files (not sure which), there's a very annoying display bug in the jj split UI. Unfolding and folding a section doesn't clear parts of the text, which makes the whole UI kind of unusable in most cases.

Probably best seen in the video below.

Steps to Reproduce the Problem

  1. Have changes in your working tree.
  2. jj split
  3. Unfold and fold a larger file.

Expected Behavior

Actual Behavior

2024-07-01_15h26_09.mp4

Specifications

  • Platform: Ubuntu 24.04
  • Version: 0.18.0
@PhilipMetzger PhilipMetzger added the 🐛bug Something isn't working label Jul 1, 2024
@arxanas
Copy link
Contributor

arxanas commented Jul 1, 2024

Hi @fschoenm, does the file in question have hard tabs (\t), or does it use spaces for indentation?

From the output, it looks like tabs to me. (Thanks for including the recording. An idle thought: perhaps we should also encourage users in the bug report template to include a recording or Asciinema screencast when demonstrating certain UI issues.)

If tabs, it may be the same problem as arxanas/scm-record#2, recently fixed by @firestack in arxanas/scm-record#37.

If it's the same, then we might want to push a new release for scm-record and pick it up in jj, since others have run into the same issue as well, like #3944.

@fschoenm
Copy link
Author

fschoenm commented Jul 1, 2024

@arxanas I'm pretty sure it's tabs but I can check it tomorrow.

Thanks for the reference to the other ticket; I tried to find similar issues but didn't know what exactly to search for.

@arxanas
Copy link
Contributor

arxanas commented Jul 1, 2024

Thanks for the reference to the other ticket; I tried to find similar issues but didn't know what exactly to search for.

It's no trouble. I probably should have also reopened the other issue, since the underlying bug has not yet been fixed and merged into jj.

@arxanas arxanas added the scm-record Issues relating to the scm-record library, used as the default interactive diff/merge editor. label Jul 1, 2024
@arxanas arxanas changed the title Display bug in jj split UI (not clearing text) Built-in diff editor doesn't redraw screen correctly in the presence of tabs Jul 1, 2024
@firestack
Copy link

Just confirming here, I was having some issues with jj resolve in a repo so I built JJ with the latest from scm-record and it worked for tabs 👍🏻

image

@msuozzo
Copy link

msuozzo commented Oct 11, 2024

What's the status here? Does this just need a version bump and release?

@martinvonz
Copy link
Member

Sounds like it to me. scm-record 0.4.0 was released yesterday. Can you try upgrading jj to that and send a PR, @msuozzo? Or we can just wait a day and hope that Dependabot will send a PR.

firestack added a commit to firestack/jj that referenced this issue Oct 14, 2024
@firestack
Copy link

#4639

@arxanas arxanas linked a pull request Oct 14, 2024 that will close this issue
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛bug Something isn't working scm-record Issues relating to the scm-record library, used as the default interactive diff/merge editor.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants