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

Allow goto_prev_buffer OR goto_previous_function... #12931

Open
nouritsu opened this issue Feb 21, 2025 · 4 comments
Open

Allow goto_prev_buffer OR goto_previous_function... #12931

nouritsu opened this issue Feb 21, 2025 · 4 comments
Labels
C-enhancement Category: Improvements

Comments

@nouritsu
Copy link

In https://github.com/helix-editor/helix/blob/master/helix-term/src/commands.rs

I noticed that the goto_previous_buffer command is not named with respect to the other goto_prev_ commands like goto_prev_function, goto_prev_class.

I think goto_previous_buffer should be changed to goto_prev_buffer to be consistent with other commands. For backward's compatibility sake, goto_previous_buffer can be aliased to the newgoto_prev_buffer.

If this is even a valid enhancement, I would like it to be my first contribution to Helix.

@nouritsu nouritsu added the C-enhancement Category: Improvements label Feb 21, 2025
@nouritsu nouritsu changed the title Allow goto_prev_buffer and allow_previous_function... Allow goto_prev_buffer and goto_previous_function... Feb 21, 2025
@nouritsu nouritsu changed the title Allow goto_prev_buffer and goto_previous_function... Allow goto_prev_buffer OR goto_previous_function... Feb 21, 2025
@oxcrow
Copy link
Contributor

oxcrow commented Feb 22, 2025

I think goto_previous_buffer should be changed to goto_prev_buffer to be consistent with other commands.

I think goto_previous_buffer is clear and easy to understand.

The other commands which use short acronyms like prev should be changed to be previous so that everything is consistent and is easy to understand by everyone.

My reasoning is this:

  • When someone reads commands like goto_prev_something we will need additional documentation to tell them what does it mean.
  • However goto_previous_something is more clear and easy to understand.

Using long descriptive names is better than using short acronyms.

@nouritsu
Copy link
Author

Is the aliasing solution viable? And how would the deprecation be handled?

@nik-rev
Copy link
Contributor

nik-rev commented Feb 27, 2025

Is the aliasing solution viable? And how would the deprecation be handled?

I'm not sure there's a need to make a breaking change just because of naming inconsistency. For example, some themes in Helix use kebab-case like adwaita-dark but others use snake_case such as boo_berry

Despite the inconsistency, it was decided not to rename these themes. It would be a breaking change (breaking change in Helix means people's configs will require editing)

@nouritsu
Copy link
Author

nouritsu commented Mar 1, 2025

Ah, I understand. I would like to start working on this, is there a mentorship system like there is for the Rust project? I'm not too sure where to start, maybe https://github.com/helix-editor/helix/blob/master/helix-term/src/commands.rs ?

Maybe by adding something like -

/* Add it to static commands */

fn move_prev_word_start(cx: &mut Context) {
    move_word_impl(cx, movement::move_prev_word_start)
}

fn move_previous_word_start(cx: &mut Context) {
    move_prev_word_start(cx)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-enhancement Category: Improvements
Projects
None yet
Development

No branches or pull requests

3 participants