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

Make some helpers for views navigate #70

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

Conversation

sinkevichmm
Copy link

added functions BufferLinePosition, SetViewLineUp, SetViewLineDown and example.

@sinkevichmm sinkevichmm changed the title make some helpers for views navigate Make some helpers for views navigate Sep 9, 2020
@glvr182
Copy link
Member

glvr182 commented Sep 10, 2020

Thanks for the pr! Ill test your code tomorrow, so far it looks good!

@sinkevichmm
Copy link
Author

Thanks for the pr! Ill test your code tomorrow, so far it looks good!

Ok!)

@glvr182
Copy link
Member

glvr182 commented Sep 16, 2020

Okay so i havent gotten around to it... I'm a bit busy right now so ill try to get around to it. Maybe @mjarkk can take a look at it

 fixed delete of rune when navigating with the mouse
@glvr182
Copy link
Member

glvr182 commented Sep 26, 2020

When i run your example, on [3] the cursor value changes, but the cursor does not move. Is this the intended behaviour?

@sinkevichmm
Copy link
Author

When i run your example, on [3] the cursor value changes, but the cursor does not move. Is this the intended behaviour?

Yes, this is an example with a wrap line that allows you to correctly display in view "props". The cursor has moved to a new line in the view array, but it is on the same line in the buffer array.

@glvr182
Copy link
Member

glvr182 commented Oct 27, 2020

I mean that would of course be the expected result, however I fear that the tools that rely on Gocui are now expecting it to work wrong and would be confused if suddenly it worked right


I have been less active in the dev world since I'm very busy with university right now, so replies can take longer than expected

@aliakseiz
Copy link

@sinkevichmm thank you for this PR.
Your functions SetViewLineUp and SetViewLineDown helped me to make the scrolling work again, as it was in the forked gocui.
awesome-gocui spoiled it completely.

@glvr182 any progress on the review?

@glvr182
Copy link
Member

glvr182 commented Jun 7, 2021

Going to ask @mjarkk to take this

Copy link
Member

@mjarkk mjarkk left a comment

Choose a reason for hiding this comment

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

Ah was gone for a while but here i am :)
Have a few questions and suggested changes.
Also can you fix the merge conflict

@@ -225,6 +236,27 @@ func (v *View) SetCursor(x, y int) error {
return nil
}

// SetViewLineUp sets the cursor position of the view at the one line up
func (v *View) SetViewLineUp() {
if v.cy > 0 {
Copy link
Member

Choose a reason for hiding this comment

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

Should'd we add an extra v.Editable check here?

func (v *View) SetViewLineDown() {
_, maxY := v.Size()
if v.cy+v.oy < v.ViewLinesHeight()-1 {
if v.cy >= maxY-1 {
Copy link
Member

Choose a reason for hiding this comment

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

Should'd we add an extra v.Editable check here?

// SetViewLineDown sets the cursor position of the view at the one line down
func (v *View) SetViewLineDown() {
_, maxY := v.Size()
if v.cy+v.oy < v.ViewLinesHeight()-1 {
Copy link
Member

Choose a reason for hiding this comment

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

Can you change this code to be like this, so we don't have nested ifs:

Suggested change
if v.cy+v.oy < v.ViewLinesHeight()-1 {
if v.cy+v.oy >= v.ViewLinesHeight()-1 {
return
}

}

}
v.lines[y] = append(v.lines[y][:x], v.lines[y][x+1:]...)
Copy link
Member

Choose a reason for hiding this comment

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

Have you tested this with half width characters?
For example:

半角

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.

None yet

4 participants