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

Convert s commands to newshell #375

Merged
merged 1 commit into from
Jan 20, 2021
Merged

Convert s commands to newshell #375

merged 1 commit into from
Jan 20, 2021

Conversation

ret2libc
Copy link
Member

@ret2libc ret2libc commented Jan 16, 2021

Your checklist for this pull request

Detailed description

  • Rename sj/s* and move them under sH. Remove s=
  • Use sd for "seek delta", which moves relative to the current offset
  • Enforce syntax. So no more s+16 but s +16. Command
    sHr is for redo history, sHu for undo history. If one wants to move
    relative to the current address, he needs to do sd +10/sd -10.
  • Remove ss commands in favour of cmd.seek.silent. This was anyway
    used mainly for scripts, so no need to replicate all s commands
    under ss.

Test plan

Try all various s commands, visual mode undo/redo.

Closing issues

Copy link
Member

@XVilka XVilka left a comment

Choose a reason for hiding this comment

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

Looks good. BTW visual mode code is horrid.

@ITAYC0HEN ITAYC0HEN self-requested a review January 16, 2021 20:07
@XVilka XVilka added this to the 0.1.0 milestone Jan 18, 2021
@wargio
Copy link
Member

wargio commented Jan 19, 2021

This might really need to be squashed hard.

@ret2libc
Copy link
Member Author

Damn, my bad.

* Rename sj/s* and move them under sH. Remove s=
* Rename s-* to sH-
* Use sd for "seek delta", which moves relative to the current offset
* Enforce <cmd> <arg> syntax. So no more "s+16" but "s +16". Command
  "sHr" is for redo history, "sHu" for undo history. If one wants to move
  relative to the current address, he needs to do "sd +10"/"sd -10".
* Remove `ss` commands in favour of cmd.seek.silent. This was anyway
  used mainly for scripts, so no need to replicate all `s` commands
  under `ss.

Change a bit rz_core_seek API to avoid dupped entries in seek history
@ret2libc ret2libc marked this pull request as ready for review January 19, 2021 16:27
Copy link
Member

@Maijin Maijin left a comment

Choose a reason for hiding this comment

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

Did you see those cmd0 with so-1 in agraph.c?

Copy link
Member

@XVilka XVilka left a comment

Choose a reason for hiding this comment

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

I suggest to merge this as is and lets test more manually. I bet not everything is caught with automated tests.

@XVilka XVilka merged commit 144ba63 into dev Jan 20, 2021
@XVilka XVilka deleted the seek-newshell branch January 20, 2021 02:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
User Experience
  
Awaiting triage
Development

Successfully merging this pull request may close these issues.

None yet

4 participants