peterh / liner

Pure Go line editor with history, inspired by linenoise
MIT License
1.05k stars 132 forks source link

Feature/configurable word separtor #113

Open reinerRubin opened 6 years ago

reinerRubin commented 6 years ago

Hello again, Sorry for the spam.

It's another variant of custom word separators. The previous pull request: https://github.com/peterh/liner/pull/112

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

peterh commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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.