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

Fixed Cursor Bug. #92

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion src/mruby-zest/mrblib/util.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,14 @@ def initialize(vg, string, width, height)
calc_cursor_x
end

def calc_cursor_x()
Copy link
Contributor

Choose a reason for hiding this comment

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

There is already a method called calc_cursor_x on line 234, which means that one will shadow the other. And this particular definition of the method is also unused (you're not calling it in TextEdit.qml). You can drop the entire method then.

return @cursor_row
end

def calc_cursor_y()
Copy link
Contributor

Choose a reason for hiding this comment

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

This method's name is confusing because it doesn't do what it says (there's no calculation involved, and 'y' is a bit ambiguous).
In that case you're returning @cursor_row, so a better name would be get_cursor_row. But Ruby makes it possible to shorten it further by simply using the same name as the attribute:

def cursor_row
  @cursor_row
end

This technique is used line 254 with pos.

See also https://ruby-doc.org/docs/ruby-doc-bundle/UsersGuide/rg/accessors.html

return @cursor_row
end

def check_args(vg, string, width, height)
if(!([Float,Integer].include? width.class))
raise TypeError.new("Invalid width <#{width.inspect}> expected Integer/Float, got #{width.class}")
Expand Down Expand Up @@ -123,6 +131,7 @@ def string_to_stats
@chrcls << :space
elsif(c == "\n" || c == "\r")
@chrcls << :line

Copy link
Contributor

Choose a reason for hiding this comment

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

This blank line does not seem to add much to the readability of the code and makes the overall diff longer. I think you can just drop that line.

else
@chrcls << :chr
end
Expand All @@ -142,7 +151,6 @@ def string_to_lines
push_char(@string[i], @widths[i])
elsif(@chrcls[i] == :line)
flush_word_buffer()

@line_widths[@active_line] = @lastw
@lastw = @activew = 0
@active_line += 1
Expand Down
7 changes: 5 additions & 2 deletions src/mruby-zest/qml/TextEdit.qml
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,18 @@ Widget {
pos = self.label.length
pos = @edit.pos if @edit
ll = self.label
size = ll.split(" ").length
Copy link
Contributor

Choose a reason for hiding this comment

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

This defines the variable size which is then unused. You can drop the entire line then.

line = @edit.calc_cursor_y
Copy link
Contributor

Choose a reason for hiding this comment

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

Some comment on the variable name: I tend to interpret line as meaning "the content of one line", instead of a number of lines. I think it'd be better named cursor_row in that case.

puts @cursor[1]
Copy link
Contributor

Choose a reason for hiding this comment

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

This is debugging code, right? Worth dropping it.

if(k.ord == 8)
pos -= 1
if(pos >= ll.length)
self.label = ll.slice(0, ll.length-1)
elsif(pos >= 0)
self.label = ll.slice(0, pos) + ll.slice(pos+1, ll.length)
self.label = ll.slice(0, pos+line) + ll.slice(pos+1+line, ll.length)
end
else
self.label = ll.insert(pos, k)
self.label = ll.insert(pos+line, k)
end
ll = self.label
self.valueRef.value = ll if self.valueRef
Expand Down