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

feat(commands): add meow-pop-or-unpop-to-mark #690

Merged
merged 1 commit into from
Dec 24, 2024

Conversation

45mg
Copy link
Contributor

@45mg 45mg commented Dec 4, 2024

Since meow-search allows us to search in the other direction using - (negative-argument), we should have similar behavior for the command that pops the mark: - will unpop the mark.

@@ -1560,10 +1560,26 @@ To search backward, use \\[negative-argument]."
Before jump, a mark of current location will be created."
(interactive)
(meow--cancel-selection)
(unless (member last-command '(meow-pop-to-mark-command meow-unpop-to-mark-command))
(unless (member last-command '(meow-pop-to-mark-command meow-unpop-to-mark-command meow-pop-or-unpop-to-mark))
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't meow-pop-to-mark-command and meow-unpop-to-mark-command non-existent commands? I noticed this when I wrote #679, but didn't change it beacuse I thought about making a new PR.

Copy link
Contributor Author

@45mg 45mg Dec 14, 2024

Choose a reason for hiding this comment

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

Introduced in e2ff708 by @DogLooksGood: did you mean to write meow-pop-to-mark and meow-unpop-to-mark, or was this just a typo?

Copy link
Contributor

Choose a reason for hiding this comment

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

did you mean to write meow-pop-to-mark and meow-unpop-to-mark, or was this just a typo?

Sorry, this was just to make an observation on something I thought could be an issue.

I'll open up a separate issue for this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(issue opened for this: #697)

@DogLooksGood
Copy link
Collaborator

What's the progress on this PR?

@gs-101
Copy link
Contributor

gs-101 commented Dec 18, 2024

What's the progress on this PR?

It seems good to me, it's just that the reference to my/pop-or-unpop-mark-command-repeat-unpop needs to be removed. I'm not sure how to do this, but I think you can even change it yourself (I had an author commit to my branch's PR once).

@45mg
Copy link
Contributor Author

45mg commented Dec 20, 2024

What's the progress on this PR?

It seems good to me, it's just that the reference to my/pop-or-unpop-mark-command-repeat-unpop needs to be removed. I'm not sure how to do this, but I think you can even change it yourself (I had an author commit to my branch's PR once).

Oh, that's a typo; should have been meow-pop-or-unpop-to-mark-repeat-unpop. I'll force-push to fix it in a moment.

Since `meow-search` allows us to search in the other direction using `-`
(`negative-argument`), we should have similar behavior for the command
that pops the mark: `-` will unpop the mark.
@45mg 45mg force-pushed the meow-pop-or-unpop-to-mark branch from 84c4141 to 5471d1a Compare December 20, 2024 04:35
Copy link
Contributor

@gs-101 gs-101 left a comment

Choose a reason for hiding this comment

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

All good now 👍

meow-command.el Outdated

See also `meow-pop-or-unpop-to-mark-repeat-unpop'."
(interactive "p")
(if (or (and my/pop-or-unpop-mark-command-repeat-unpop
Copy link
Contributor

Choose a reason for hiding this comment

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

You forgot to remove the my/ prefix.

-  (if (or (and my/pop-or-unpop-mark-command-repeat-unpop
+  (if (or (and meow-pop-or-unpop-to-mark-repeat-unpop

Copy link
Contributor

Choose a reason for hiding this comment

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

Just this one needs to be fixed 👍

@@ -1560,10 +1560,26 @@ To search backward, use \\[negative-argument]."
Before jump, a mark of current location will be created."
(interactive)
(meow--cancel-selection)
(unless (member last-command '(meow-pop-to-mark-command meow-unpop-to-mark-command))
(unless (member last-command '(meow-pop-to-mark-command meow-unpop-to-mark-command meow-pop-or-unpop-to-mark))
Copy link
Contributor

Choose a reason for hiding this comment

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

did you mean to write meow-pop-to-mark and meow-unpop-to-mark, or was this just a typo?

Sorry, this was just to make an observation on something I thought could be an issue.

I'll open up a separate issue for this.

@DogLooksGood DogLooksGood merged commit 43bc65a into meow-edit:master Dec 24, 2024
2 checks passed
jidibinlin pushed a commit to jidibinlin/meow that referenced this pull request Jan 2, 2025
Since `meow-search` allows us to search in the other direction using `-`
(`negative-argument`), we should have similar behavior for the command
that pops the mark: `-` will unpop the mark.

Co-authored-by: 45mg <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants