This PR sends the cursor to the beginning or end of the document when "go to line" is out of bounds. When this occurs, the line marker is not set. Behavior of both the dialog and toolbar are affected. Addresses #2146. You can view, comment on, or merge this pull request online at:
https://github.com/geany/geany/pull/2973
-- Commit Summary --
* <a href="https://github.com/geany/geany/pull/2973/commits/9cd5a290093c76d57c77653a08340739b47d1ea5">When "go to line" is out of bounds, go to beginning or end of document</a>
-- File Changes --
M src/editor.c (32)
-- Patch Links --
https://github.com/geany/geany/pull/2973.patch https://github.com/geany/geany/pull/2973.diff
Since it is intended that the line number is always positive (and it is in both cases where `editor_goto_line()` is called) and the sign is communicated in `offset` it is a programming error if `line_no < 0` so it should use one of the glib checks that log a warning, eg `g_return_val_if_fail()`. Have a look at the usage elsewhere in Geany since Glib doesn't bother to document it any more.
Also its really only needed to check for out of range after the offset calculation, checking before and setting to max will still give an out of range result (for any current line except 0) so no need to check before.
@xiota pushed 1 commit.
f035c1ede49f4c02683112cd0230a58374510879 change bounds check
@elextr Thanks. Changed as you've suggested.
@xiota pushed 1 commit.
8fc12dc9d277208a034c5d770e537429fbbe161e remove unnecessary initialization
@xiota pushed 1 commit.
d00212d5ca78bd59b1bacd3b17e26c5bc1461371 add comments
Tested ok, except if the user enters 0 gets the warning: "(geany:6625): Geany-CRITICAL **: 10:51:24.814: editor_goto_line: assertion 'line_no >= 0' failed"
Scintilla docs for `gotoline` say numbered from 0 and thats the only place I can find that explicitly specifies it.
But the margins are numbered from 1 and entering a number goes to the start of the line with that margin number. Geany uses `getposfromline` and `gotopos` not `gotoline` so maybe that compensates in some way????? I'm confused.
Anyway maybe its better to only warn on < 0 and silently change 0 to 1 rather than give a warning for user input of 0.
Update:
Found https://github.com/geany/geany/blob/04566236d3f9811d55d6c7bcc492b3a60369fd20... that makes line numbers 0 based when passed to `editor_goto_line()` so just needs to change the warning test to < 0.
@elextr Would you have any objection to moving the `-1` from `get_line_and_offset_from_text` to `editor_goto_line` so that `line_no` is always non-negative as previously expected?
As far as I can tell, these functions are used only for this feature, and not by any plugins.
Another option is to make `offset` a bool and keep the sign as part of `line_no`.
Would you have any objection to moving the -1 from get_line_and_offset_from_text to editor_goto_line so that line_no is always non-negative as previously expected?
Since the function that decodes the text value is `static` its only available for use inside callbacks.c so making it return the number the user input for both absolute and offset cases is more consistent and less accident prone than returning the number for offset and the number minus one for absolute, so this is fine.
`editor_go_to_line()` isn't in the plugin API, so its only usable inside Geany, so making it one based is also fine, but probably best to add a comment above it to say that so we don't go through this again.
@xiota pushed 1 commit.
662ae7e91e76fd605300a21098899dd1da07539a change offset to gboolean in editor_goto_line()
@xiota pushed 1 commit.
aa32cfa253bf5ffee032912c6d95d46ef62cbca3 update function declaration
@xiota pushed 1 commit.
113872283a4339cf947320ba8121ecf621773839 use gint for line_count
I changed `offset` to bool and kept the sign with `line_no` to see what it looks like.
* Calculating the destination line number is simplified. No more multiplying by `offset` (which is confusing). * `get_line_and_offset_from_text()` became a two-liner, so I removed it.
Havn't looked in detail, but the idea seems sound, and one less weird little local function is a good thing IMHO :grin:
Doxygen is used to generate the plugin API docs, so its not used for Geany internal function documentation, so the normal "style" is to not use doxygen markup in comments on internal functions, and to keep it brief (ok, nonexistant in most cases), only mentioning the key things like line_no is 1 based.
@xiota pushed 1 commit.
c2786f567938d5e512368d2306bd03dbe3e428e5 change comment, formatting
the normal "style" is to not use doxygen markup in comments on internal functions, and to keep it brief
Thanks... changed.
@xiota pushed 1 commit.
e11bc7bea336871625f1ea7a6daca4258bcb4095 reduce code duplication
Merged #2973 into master.
WFM thanks.
WFM = Will Finally Merge?
Works For Me
@elextr Thanks... Too many abbreviations to keep track of...
github-comments@lists.geany.org