-
Notifications
You must be signed in to change notification settings - Fork 346
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
Fix empty files can't be selected in builtin diff editor #3481
Conversation
This is a first pass at fixing #3016. The upstream This PR should be a complete fix for |
4d92b16
to
6b7cf29
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but @arxanas might have better suggestion.
fef2509
to
ef95706
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. There are no other comments, so I went ahead and flagged as approved.
ef95706
to
963c6de
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for fixing!
Thanks for reviewing @arxanas! |
963c6de
to
0e85704
Compare
I'm going to submit this now and work on a patch for |
Sounds good. Thanks, both of you! EDIT: all three of you! |
0e85704
to
1b41780
Compare
This fixes several issues that made working with empty files difficult using the builtin diff editor. 1. The scm-record library used for the editor expects each file to have at least one section. For new empty files this should be a file mode section. jj wasn't rendering this mode section, which prevented empty files from being selected at all. 2. The scm-record library returns `SelectedContents::Absent` if the file has no contents after the diff editor is invoked. This can be because of several reasons: 1) the file is new and empty; 2) the file was empty before and is still empty; 3) the file has been deleted. Perhaps this is a bug in scm-record and it should return `SelectedContents::Unchanged` or `SelectedContents::Present` if the file hasn't been deleted. Until this is patched upstream, we can work around it by disambiguating these cases. See arxanas/scm-record#26 for the upstream bug. Fixes #3016
1b41780
to
09df602
Compare
This fixes several issues that made working with empty files difficult using the builtin diff editor.
The scm-record library used for the editor expects each file to have at least one section. For empty files this should be a file mode section. jj wasn't rendering this mode section, which prevented empty files from being selected at all.
The scm-record library returns
SelectedContents::Absent
if the file has no contents after the diff editor is invoked. This can be because of several reasons: 1) the file is new and empty; 2) the file was empty before and is still empty; 3) the file has been deleted. Perhaps this is a bug in scm-record and it should returnSelectedContents::Unchanged
orSelectedContents::Present
if the file hasn't been deleted. Until this is patched upstream, we can work around it by disambiguating these cases.See arxanas/scm-record#26 for the upstream bug.
Fixes #3016