Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Edit mode and input byte insertion#503

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
connor4312 merged 13 commits intomicrosoft:mainfromtomilho:fr-edit
Apr 15, 2024
Merged

Conversation

@tomilho
Copy link
Contributor

Opening a PR for some early feedback on the feature.

Introduces input byte insertion and edit mode to switch between insertion and replacing.

To choose between these, there is a edit mode that can be changed either in the status bar or through a command.
editselect

To visually distinguish them inside the editor, each mode has an unique cursor on the focusedByte. When Inserting, the cursor is a text cursor and in Replacing, it is a box.

Inserting
insertedit

Replacing
replaceedit

At the moment, it is not possible to insert at the end of the file and there are some annoying bugs. But, hopefully, it provides a good picture on how input byte insertion/replacing can be.

clnhlzmn and connor4312 reacted with hooray emoji
Allows to manually insert bytes into the file, just like replace. Tochoose between these, there is a edit mode that can be changedeither in the status bar or through a command.To visually distinguish them inside the editor, each mode has an uniquecursor on the focusedByte. When Inserting the cursor is a text cursorand in Replacing it is a box.
Changed cursor color in insertion and replacement to match editor's cursor
Initially was using a text cursor to append at a certain offset, but itwas terrible UX due to having to specifically click on the rhs of the offset.So I opted to add an extra data cell at the end with a "+" icon. This way the usercan easily figure out how to append bytes and does not complicate byte insertion.
Fixed inserting an invalid key shifts the focusedElementRefactored newValue logic to use parseHexDigit
@connor4312
Copy link
Member

Some comments, great job with this PR though, it looks super cool. Let me know if you want help with append behavior.

@tomilho
Copy link
ContributorAuthor

Thank you for the taking the time to comment! I will carefully go through all of them tomorrow as it is getting late here. At the moment, I am not entirely sure if it is a good idea to have<AppendDataCell> separate to<DataCell> as they share many common behavior. Should I join them together or keep them separate? Thank you for the help!

@connor4312
Copy link
Member

I think it would probably be easier to try to join them together, since there's a lot of overlap between them. But I may be wrong, I'm not convicted one way or the other at this point.

tomilho reacted with thumbs up emoji

@tomilho
Copy link
ContributorAuthor

Apologies for the late message. I have gone through all the comments, so I hope the code is better now. At the moment, my biggest concern is related to#511 since, with this feature, it has become a lot easier to encounter it. I will try to figure out what is causing before opening the PR for merge.

Just want to say, again, thank you for the comments. They implicitly answered some questions I was having.

@tomilho
Copy link
ContributorAuthor

I have somewhat figured out what is causing#511 , so I think we can merge this. I am still working on a proper fix, as the problem is rather complex.

Copy link
Member

@connor4312connor4312 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

This PR looks great. Could hardly have done it better myself :) Just a few small comments but then this should be good to merge

tomilho reacted with laugh emoji
@vscodenpavscodenpa added this to theApril 2024 milestoneApr 15, 2024
@connor4312connor4312 merged commit7723cef intomicrosoft:mainApr 15, 2024
@tomilho
Copy link
ContributorAuthor

Thank you for the help and approving!

connor4312 reacted with heart emoji

@tomilhotomilho deleted the fr-edit branchApril 15, 2024 18:11
@GitMensch
Copy link
Contributor

If there any chance to provide that for normal editors, giving the long standing (2015)microsoft/vscode#1012?

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@TyriarTyriarTyriar approved these changes

@connor4312connor4312connor4312 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

April 2024

Development

Successfully merging this pull request may close these issues.

5 participants

@tomilho@connor4312@GitMensch@Tyriar@vscodenpa
[8]ページ先頭
©2009-2025 Movatter.jp