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

Feature/configurable word separtor #113

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

reinerRubin
Copy link

Hello again,
Sorry for the spam.

It's another variant of custom word separators. The previous pull request: #112

Here I tried to add "SetWhitespaceFunc" from an old proposal.

@peterh
Copy link
Owner

peterh commented Oct 1, 2018

Please don't worry about spam. Thank you for taking the time to create multiple patches for discussion.

While looking at your previous patch, I noticed that bash treats punctuation differently for "cursor" operations compared to "delete word" operations. So we need (at least) two different functions.

And thinking about it, a single rune test is insufficient for more complex use cases. IIRC the original feature request wanted to configure CamelCase as separate words (and presumably ALLCAPS is a single word, not seven words). Maybe someone wants to treat . as a separator when it's part of a .. (string concat operator), but not when it's part of an integer literal (eg. 3.1416). So I wonder if it might be better to expose overrides as func ([]rune, int) DeleteWord(line []rune, pos int), int NextWord(line []rune, pos int) and PrevWord (or similar), with default versions of those functions that provide the bash behaviour. I'm interested to hear your thoughts.

(As a minor style nit, _ in file names is used for magic build flags like _windows or _test. I'd be happier if we avoided underscores except when used for a magic build flag).

@reinerRubin reinerRubin force-pushed the feature/configurable-word-separtor branch from b84b4fd to 82b1ad2 Compare October 1, 2018 16:30
@reinerRubin
Copy link
Author

reinerRubin commented Oct 1, 2018

Hm, I have not noticed the difference between ctrl-w and alt-f behavior in bash before. Yeah, "nextWorld", "deleteWord" and so on make sense. Should these functions do all the job or just return something like:

struct {
  cut: from, to // optional
  newPosition: // "after cur"
}

?

I prefer the second variant, because these functions would be without side effects on the main state, so it would be easier to write them and test.

@reinerRubin
Copy link
Author

reinerRubin commented Oct 2, 2018

Ok, I tried to group the word related functions into an interface and added an effect entity. It will allow to add some tests for these functions. I added the default implementation and the bash-like.

What do you think? If this approach is ok, I'll cover these methods by tests and make rebase.

// idk, maybe they should take a copy of the line.

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.

2 participants