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

Add traver through url #29

Closed
wants to merge 5 commits into from
Closed

Add traver through url #29

wants to merge 5 commits into from

Conversation

Eraden
Copy link
Contributor

@Eraden Eraden commented Feb 7, 2017

Related to #27

@asciimoo
Copy link
Owner

asciimoo commented Feb 7, 2017

Great! However it would be even better to apply these to all editable views. Another approach is to extend the default ViewEditor's Edit function (https://github.com/asciimoo/wuzz/blob/master/wuzz.go#L105) with the new features and it will be automatically applied to all the input views.

…onal operators.

Extend ViewEditor Edit method.
Underscore redundant variables.
wuzz.go Outdated
@@ -149,7 +133,12 @@ func (e *ViewEditor) Edit(v *gocui.View, key gocui.Key, ch rune, mod gocui.Modif
e.backTabEscape = true
return
}

if key == gocui.KeyHome {
_ = e.GoToHome(v)
Copy link
Owner

Choose a reason for hiding this comment

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

_ = is not necessary.
Another thing: isn't return required after these statements? Without returning the original editor function will be called as well. Alternatively it can be turned into a switch-case statement like this:

switch key {
case gocui.KeyHome:
    e.GoToHome(v)
default:
    e.origEditor.Edit(..)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What exactly is backTabEscape?

Copy link
Owner

Choose a reason for hiding this comment

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

@Eraden there is no such thing "backtab" or "shift+tab" in terminal, if you press shift+tab it generates the sequence \033]Z. The purpose of this function is to catch these character sequences. backTabEscape is a boolean state flag, which is true if the previous key was the first part of the "backtab" sequence.

.gitignore Outdated
@@ -0,0 +1 @@
wuzz
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't seem appropriate as part of this commit to me.

Copy link
Owner

Choose a reason for hiding this comment

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

Agree, @Eraden could you open another PR with this change?

wuzz.go Outdated
@@ -164,7 +153,7 @@ func (e *SearchEditor) Edit(v *gocui.View, key gocui.Key, ch rune, mod gocui.Mod
func (a *App) Layout(g *gocui.Gui) error {
maxX, maxY := g.Size()
if maxX < MIN_WIDTH || maxY < MIN_HEIGHT {
if v, err := g.SetView("error", 0, 0, maxX-1, maxY-1); err != nil {
if v, err := g.SetView("error", 0, 0, maxX - 1, maxY - 1); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these formatting changes in the same commit as the above functional changes? This makes for some pretty messy history, IMO.

Copy link
Contributor Author

@Eraden Eraden Feb 7, 2017

Choose a reason for hiding this comment

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

True, I will reverse it and squash commit.

@@ -832,7 +818,7 @@ func setViewTextAndCursor(v *gocui.View, s string) {
v.SetCursor(len(s), 0)
}

func quit(g *gocui.Gui, v *gocui.View) error {
func quit(_ *gocui.Gui, _ *gocui.View) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

I personally don't see the value in eliding the argument names here, but that's a style call that @asciimoo should probably make the final decision on.

Copy link
Owner

Choose a reason for hiding this comment

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

I suppose this is the proper go way, but go fmt does not correct it.
This mod is OK to me in another PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

KK

@asciimoo
Copy link
Owner

Thank you the PR and your work, but another Implementation of this functionality has been merged: #39

@asciimoo asciimoo closed this Feb 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants