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

Disable multiline on url and method views #39

Merged
merged 2 commits into from
Feb 10, 2017
Merged

Conversation

cyberj
Copy link
Contributor

@cyberj cyberj commented Feb 8, 2017

  • Disable multiline on url, method & search views by creating a singlelineEditor
  • Add HOME END shortcuts to theses views to jump at beginning and end of line according to Support for END and HOME key #27
  • Arrow up & down on url and search view jumps at beginning and end of line
  • Arrow up & down on method view change the method in the same order of the method-list popup

Copy link
Contributor

@Benaiah Benaiah left a comment

Choose a reason for hiding this comment

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

This looks good, and works well on initial examination. I particularly like the up/down scrolling in the "method" view - that's really handy.

Is there any reason not to include the search field as well? I don't see why that should be multiline, as the inability to see more than one line makes it very confusing when you inadvertently go to a new line. \n can be used to make the regex match multiple lines, and doesn't have the same problem.

If that's an unintentional omission, I would suggest adding a commit that applies your singlelineEditor to that field.

Thanks for the PR! It's definitely something that was needed.

@@ -19,7 +19,7 @@ $ "$GOPATH/bin/wuzz" --help
### Commands

Keybinding | Description
----------------------------------------|------------------------------------------------------------
----------------------------------------|---------------------------------------
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this is too nitpicky, but this change doesn't seem to fit with this commit or 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.

Yeah, that was just a fix to keep the README at 79cols. I don't think it need a commit on its own, but i can split it if needed

e.wuzzEditor.Edit(v, key, ch, mod)
}

//
Copy link
Contributor

Choose a reason for hiding this comment

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

This looks like it was left in accidentally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a separator with line 111 // Editor funcs

@cyberj
Copy link
Contributor Author

cyberj commented Feb 8, 2017

If that's an unintentional omission, I would suggest adding a commit that applies your singlelineEditor to that field.

That was not unintentional, because I had a bug with it, but its OK now.

wuzz.go Outdated
@@ -166,6 +207,7 @@ func (a *App) Layout(g *gocui.Gui) error {
setViewDefaults(v)
v.Editable = true
v.Title = "URL params (F3)"
v.Editor = &singlelineEditor{defaultEditor}
Copy link
Owner

Choose a reason for hiding this comment

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

It is allowed to break lines in the GET parameter view too, so I'd suggest to use just the default editor here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah but we can't use multilines requests now because Enter will launch request and will not break lines.

Copy link
Owner

Choose a reason for hiding this comment

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

Enter only works in the URL view as far as I know.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed i thought it was the url view, my bad

Copy link
Owner

@asciimoo asciimoo left a comment

Choose a reason for hiding this comment

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

Very good PR generally. Please fix that one minor editor issue

@asciimoo
Copy link
Owner

Thanks the fixes. I found another regression: it does not handle shift+tab. Still not fully clear why, if you have any idea, let me know

@cyberj
Copy link
Contributor Author

cyberj commented Feb 10, 2017

Found it : editor passed by value and not reference for e.backTabEscape

@cyberj cyberj force-pushed the master branch 2 times, most recently from 8f580a6 to bd6ca98 Compare February 10, 2017 16:12
@cyberj
Copy link
Contributor Author

cyberj commented Feb 10, 2017

Rebased and squashed (on master sorry)

@asciimoo
Copy link
Owner

Awesome! Thanks.

@asciimoo asciimoo merged commit 155336a into asciimoo:master Feb 10, 2017
This was referenced Feb 14, 2017
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