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

Fix searchterm reverts #1336

Merged
merged 12 commits into from
Jul 11, 2023
Merged

Fix searchterm reverts #1336

merged 12 commits into from
Jul 11, 2023

Conversation

jeremypw
Copy link
Collaborator

@jeremypw jeremypw commented Jun 16, 2023

Fixes #1335

  • Any search entry text now always takes priority.
  • The searchbar now listens for selection text changes and clears itself if the current search term does not match the new selection (to be tested for unwanted behaviour). This allows the user to select some text and search on it as before.
  • The selection_changed signal is now only emitted when the text actually changes (not just its position) so selecting the same text in a different position is ignored. This avoids unwanted clearance of the search term when finding the next or previous match.

The next Code release is waiting for this to be merged.

@jeremypw jeremypw requested a review from a team June 17, 2023 08:38
@jeremypw jeremypw added the Priority: High To be addressed after any critical issues label Jun 17, 2023
@jeremypw jeremypw marked this pull request as draft June 21, 2023 17:15
@jeremypw
Copy link
Collaborator Author

Converting to draft while searching for a better solution.

@jeremypw
Copy link
Collaborator Author

Will test the new solution for a while before undrafting.

@jeremypw jeremypw marked this pull request as ready for review July 5, 2023 18:10
zeebok
zeebok previously approved these changes Jul 9, 2023
Copy link
Contributor

@zeebok zeebok left a comment

Choose a reason for hiding this comment

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

Not sure if it is related, but I noticed if I hit Esc to close the search with text in the entry field, I would see this in terminal:
(io.elementary.code:3534): GLib-GObject-CRITICAL **: 12:33:30.566: g_object_notify_by_pspec: assertion 'G_IS_PARAM_SPEC (pspec)' failed
This doesn't appear to happen in the current public build. Otherwise this does indeed fix the issue.

src/Widgets/SourceView.vala Outdated Show resolved Hide resolved
@jeremypw
Copy link
Collaborator Author

Not sure if it is related, but I noticed if I hit Esc to close the search with text in the entry field, I would see this in terminal: (io.elementary.code:3534): GLib-GObject-CRITICAL **: 12:33:30.566: g_object_notify_by_pspec: assertion 'G_IS_PARAM_SPEC (pspec)' failed This doesn't appear to happen in the current public build. Otherwise this does indeed fix the issue.

@zeebok I cannot reproduce this at the moment. Maybe you could try running under gdb with set env G_DEBUG=fatal-criticals to get a backtrace? What exact steps are required to trigger this?

@zeebok
Copy link
Contributor

zeebok commented Jul 10, 2023

@zeebok I cannot reproduce this at the moment. Maybe you could try running under gdb with set env G_DEBUG=fatal-criticals to get a backtrace? What exact steps are required to trigger this?

I'll give this a try. I just would <Ctrl>F and type something and press escape.

@zeebok
Copy link
Contributor

zeebok commented Jul 10, 2023

hmm it actually also happened with it loading the previous documents. The frame isn't very insightful:

G_IS_PARAM_SPEC (pspec)' failed

Thread 1 "io.elementary.c" received signal SIGTRAP, Trace/breakpoint trap.
0x00007ffff7ecd447 in g_logv () from /lib/x86_64-linux-gnu/libglib-2.0.so.0

@jeremypw
Copy link
Collaborator Author

@zeebok That's what I am doing. I have all plugins enabled except vim emulation.

@zeebok
Copy link
Contributor

zeebok commented Jul 10, 2023

This seems to be unrelated then. I wonder if running in a VM is effecting things?

@jeremypw
Copy link
Collaborator Author

@zeebok If you type bt for a full backtrace you might find the last vala line that executed.

@jeremypw
Copy link
Collaborator Author

Maybe.

@zeebok
Copy link
Contributor

zeebok commented Jul 10, 2023

Aaah that's the command, thanks, been awhile since I have worked with gdb directly.

#1  0x00007ffff7ecd703 in g_log () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#2  0x00007ffff1008e86 in code_format_bar_set_tab_width_set_by_editor_config (self=0x5555559be9e0, value=1)
    at ../src/Widgets/FormatBar.vala:22
#3  0x00007ffff10d3ee8 in __lambda5_ (self=0x555555a28e10, d=0x55555599fc30) at ../plugins/editorconfig/editorconfig.vala:65
#4  0x00007ffff10d41d9 in ___lambda5__scratch_services_interface_hook_document
    (_sender=0x5555558d4ed0, doc=0x55555599fc30, self=0x555555a28e10) at ../plugins/editorconfig/editorconfig.vala:34
#5  0x00007ffff7c4ad2f in g_closure_invoke () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#6  0x00007ffff7c66c36 in  () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#7  0x00007ffff7c68614 in g_signal_emit_valist () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#8  0x00007ffff7c68863 in g_signal_emit () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#9  0x00005555555c39ca in __lambda121_ (self=0x555555788c20, d=0x55555599fc30) at ../src/Services/PluginManager.vala:112
#10 0x00005555555c39f7 in ___lambda121__scratch_services_plugins_manager_hook_document
    (_sender=0x555555788c20, doc=0x55555599fc30, self=0x555555788c20) at ../src/Services/PluginManager.vala:111
#11 0x00007ffff7c4ad2f in g_closure_invoke () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#12 0x00007ffff7c66c36 in  () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#13 0x00007ffff7c68614 in g_signal_emit_valist () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#14 0x00007ffff7c68a8e in g_signal_emit_by_name () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#15 0x000055555557c9f1 in __lambda155_ (self=0x555555798380, doc=0x55555599fc30) at ../src/MainWindow.vala:534
#16 0x000055555557cb5e in ___lambda155__scratch_widgets_document_view_document_change
    (_sender=0x55555595a8e0, document=0x55555599fc30, parent=0x55555595a8e0, self=0x555555798380) at ../src/MainWindow.vala:522
#17 0x00005555555d0723 in g_cclosure_user_marshal_VOID__OBJECT_OBJECT
    (closure=0x555555de0f40, return_value=0x0, n_param_values=3, param_values=0x7fffffffc9d0, invocation_hint=0x7fffffffc950, marshal_data=0x0) at ../src/Widgets/DocumentView.vala:21
#18 0x00007ffff7c4ad2f in g_closure_invoke () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#19 0x00007ffff7c66c36 in  () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#20 0x00007ffff7c68614 in g_signal_emit_valist () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#21 0x00007ffff7c68863 in g_signal_emit () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#22 0x00005555555cfb28 in scratch_widgets_document_view_on_focus_in_event (self=0x55555595a8e0) at ../src/Widgets/DocumentView.vala:406
#23 0x00005555555ceea7 in _scratch_widgets_document_view_on_focus_in_event_gtk_widget_focus_in_event
    (_sender=0x555556517c20, event=0x555555cda0d0, self=0x55555595a8e0)
    at /home/ryan/Downloads/code-fix-searchterm-reverts/build/DocumentView.c:2272
#24 0x00007ffff75e5b77 in  () at /lib/x86_64-linux-gnu/libgtk-3.so.0
#25 0x00007ffff7c4ad2f in g_closure_invoke () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#26 0x00007ffff7c66e11 in  () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#27 0x00007ffff7c68026 in g_signal_emit_valist () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#28 0x00007ffff7c68863 in g_signal_emit () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#29 0x00007ffff75ae724 in  () at /lib/x86_64-linux-gnu/libgtk-3.so.0
#30 0x00007ffff75ada35 in gtk_widget_send_focus_change () at /lib/x86_64-linux-gnu/libgtk-3.so.0
#31 0x00007ffff75bbf48 in  () at /lib/x86_64-linux-gnu/libgtk-3.so.0
#32 0x00007ffff75bc63a in  () at /lib/x86_64-linux-gnu/libgtk-3.so.0
--Type <RET> for more, q to quit, c to continue without paging--
#33 0x00007ffff7c49745 in g_cclosure_marshal_VOID__OBJECTv () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#34 0x00007ffff7c68700 in g_signal_emit_valist () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#35 0x00007ffff7c68863 in g_signal_emit () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#36 0x00007ffff7c68700 in g_signal_emit_valist () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#37 0x00007ffff7c68863 in g_signal_emit () at /lib/x86_64-linux-gnu/libgobject-2.0.so.0
#38 0x00007ffff759c50a in gtk_widget_grab_focus () at /lib/x86_64-linux-gnu/libgtk-3.so.0
#39 0x00005555555b20da in scratch_services_document_focus (self=0x55555599fc30) at ../src/Services/Document.vala:680
#40 0x00005555555cd730 in __lambda35_ (_data29_=0x555555833140, obj=0x55555599fc30, res=0x555556460e20)
    at ../src/Widgets/DocumentView.vala:240
#41 0x00005555555cd7a0 in ___lambda35__gasync_ready_callback (source_object=0x55555599fc30, res=0x555556460e20, self=0x555555833140)
    at ../src/Widgets/DocumentView.vala:237
#42 0x00007ffff7d47e39 in  () at /lib/x86_64-linux-gnu/libgio-2.0.so.0
#43 0x00007ffff7d4805b in  () at /lib/x86_64-linux-gnu/libgio-2.0.so.0
#44 0x00005555555ae37a in scratch_services_document_open_co (_data_=0x5555566668b0) at ../src/Services/Document.vala:407
#45 0x00005555555aca30 in scratch_services_document_open_ready
    (source_object=0x55555663adc0, _res_=0x555556584b40, _user_data_=0x5555566668b0) at ../src/Services/Document.vala:364
#46 0x00007ffff7d47e39 in  () at /lib/x86_64-linux-gnu/libgio-2.0.so.0
#47 0x00007ffff7d4805b in  () at /lib/x86_64-linux-gnu/libgio-2.0.so.0
#48 0x00007ffff6fe898d in  () at /lib/x86_64-linux-gnu/libgtksourceview-4.so.0
#49 0x00007ffff7d11302 in  () at /lib/x86_64-linux-gnu/libgio-2.0.so.0
#50 0x00007ffff7d47e39 in  () at /lib/x86_64-linux-gnu/libgio-2.0.so.0
#51 0x00007ffff7d47e7d in  () at /lib/x86_64-linux-gnu/libgio-2.0.so.0
#52 0x00007ffff7ec4c44 in g_main_context_dispatch () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#53 0x00007ffff7f1a258 in  () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#54 0x00007ffff7ec23e3 in g_main_context_iteration () at /lib/x86_64-linux-gnu/libglib-2.0.so.0
#55 0x00007ffff7d76fb5 in g_application_run () at /lib/x86_64-linux-gnu/libgio-2.0.so.0
#56 0x0000555555579626 in scratch_application_main (args=0x7fffffffdd28, args_length1=1) at ../src/Application.vala:166
#57 0x0000555555579673 in main (argc=1, argv=0x7fffffffdd28) at ../src/Application.vala:165

Looks like frame 2, the FormatBar?

@jeremypw
Copy link
Collaborator Author

Odd. This code does not seem to touch the FormatBar (or tab options, syntax, etc).

@jeremypw
Copy link
Collaborator Author

You could try turning off the editorconfig plugin.

@zeebok
Copy link
Contributor

zeebok commented Jul 10, 2023

You could try turning off the editorconfig plugin.

That made it go away, including in the circumstance that started this whole conversation.

@jeremypw
Copy link
Collaborator Author

Could be something to do with how plugins work in a VM or some kind of race I guess. Pressing Esc will refocus the document which will trigger a number of things. Document refocusing can be achieved just be clicking on it though - do you get the warning when the document is refocused in ways other than pressing Esc?

@jeremypw
Copy link
Collaborator Author

I notice that I have tab_set_by_editor_config on line 21 of FormatBar.vala whereas your backtrace gives line 22?

@jeremypw
Copy link
Collaborator Author

@zeebok Are you running the latest master in a VM as well? I am having difficulty finding a reason why this PR should behave differently to master regarding the FormatBar and editconfig plugin.

@zeebok
Copy link
Contributor

zeebok commented Jul 10, 2023

I notice that I have tab_set_by_editor_config on line 21 of FormatBar.vala whereas your backtrace gives line 22?

In my source it is also line 21 so I am not sure

@jeremypw
Copy link
Collaborator Author

I notice that I have tab_set_by_editor_config on line 21 of FormatBar.vala whereas your backtrace gives line 22?

In my source it is also line 21 so I am not sure

Must be a quirk of gdb I guess.

Copy link
Contributor

@zeebok zeebok left a comment

Choose a reason for hiding this comment

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

Since that error is from the plugin I am going to approve this

jeremypw pushed a commit that referenced this pull request Jul 11, 2023
@jeremypw jeremypw merged commit b58e753 into master Jul 11, 2023
12 checks passed
@jeremypw jeremypw deleted the fix-searchterm-reverts branch July 11, 2023 13:41
jeremypw pushed a commit that referenced this pull request Jul 11, 2023
* Correct links to issues and sort

* Add recent issue fixes

* Update for pull #1336
zeebok added a commit that referenced this pull request Aug 1, 2023
* Update project version in meson.build

* Release 7.1.0 update metainfo (#1350)

* Correct links to issues and sort

* Add recent issue fixes

* Update for pull #1336

* Add another fixed issue to metainfo, update date

---------

Co-authored-by: Ryan Kornheisl <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Priority: High To be addressed after any critical issues
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Search entry reverts to previous term after editing and <Ctrl>f
2 participants