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

Add CodeFind feature #364

Closed
wants to merge 28 commits into from
Closed

Add CodeFind feature #364

wants to merge 28 commits into from

Conversation

SoNiC-HeRE
Copy link
Contributor

Closes #347

@SoNiC-HeRE SoNiC-HeRE marked this pull request as draft June 25, 2023 10:10
@SoNiC-HeRE
Copy link
Contributor Author

The searchbar is not visible at all and when the searchbar is moved outside the template it becomes visible but expands vertically way too much and the codeview isnt implemented.

@SoNiC-HeRE
Copy link
Contributor Author

SoNiC-HeRE commented Jun 27, 2023

setupSearch() {
    const { buffer } = this;
    this.search_entry.connect("search-changed", () => {
      const searchContext = new Source.SearchContext(buffer);
    });
  }

initialising searchcontext throws a bunch of errors;
Using this for reference https://gnome.pages.gitlab.gnome.org/gtksourceview/gtksourceview5/ctor.SearchContext.new.html

@sonnyp
Copy link
Contributor

sonnyp commented Jun 27, 2023

@SoNiC-HeRE in GJS new X is to initialize with properties

try

new SearchContext({
  buffer
})

additionally, your code is wrong, I doubt that you're supposed to create a new SearchContext on every change

@SoNiC-HeRE
Copy link
Contributor Author

Implemented the requested layout and functionalities suggested in the #347
For the buttons: I need help with the implementation of searchContext.forward_async and searchContext.backward_async functions to iterate through the search results and move the cursor accordingly

@SoNiC-HeRE SoNiC-HeRE requested a review from sonnyp June 29, 2023 21:35
@SoNiC-HeRE
Copy link
Contributor Author

SoNiC-HeRE commented Jun 29, 2023

Implemented the requested layout and functionalities suggested in the #347 For the buttons: I need help with the implementation of searchContext.forward_async and searchContext.backward_async functions to iterate through the search results and move the cursor accordingly

For example: The forward_async method returns a boolean value if a match was found or not and also returns location for start of match and end of a match. How can I use this info to start my start my iter from first occurence and then move to the next match location iterating through the buffer and moving my cursor.

https://gnome.pages.gitlab.gnome.org/gtksourceview/gtksourceview5/method.SearchContext.forward_async.html
https://gnome.pages.gitlab.gnome.org/gtksourceview/gtksourceview5/method.SearchContext.forward_finish.html

@SoNiC-HeRE
Copy link
Contributor Author

 this.next_match.connect("clicked", () => {
      settings.search_text = this.search_entry.get_text();
      const searchContext = new Source.SearchContext({ buffer, settings });

      const currentIter = this.buffer.get_iter_at_mark(
      this.buffer.get_insert()
      );

      const [found, iter] = searchContext.forward(currentIter);
        if (found) {
        // Scroll to the previous match
          this.buffer.place_cursor(iter);
          this.buffer.move_mark_by_name("insert", iter);
        // this.source_view.scroll_mark_onscreen(iter);
        } else {
        // Handle no previous match found
        console.log("No previous match found.");
      }});

I've implemented this function to make it work such that wherever the cursor is; it get's the next searched-match location by locating highlight and moving down the cursor one step; I've commented the scroll function because it breaks code;

I'm unsure how do i modify this code to make it keep the cursor moving to next match-location not only vertically but horizontally as well

Copy link
Contributor

@sonnyp sonnyp left a comment

Choose a reason for hiding this comment

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

I pushed a commit that makes next/previous work dd3b17b make sure to git pull

This is how I figured it out:

https://gnome.pages.gitlab.gnome.org/gtksourceview/gtksourceview5/class.SearchContext.html

The concept of “current match” doesn’t exist yet. A way to highlight differently the current match is to select it.

Ha!

In the GtkSourceView source code, there is an example of how to use the search and replace API: see the tests/test-search.c file. It is a mini application for the search and replace, with a basic user interface.

I simply ported the relevant code to GJS see https://gitlab.gnome.org/GNOME/gtksourceview/-/blob/master/tests/test-search.c

Since this isn't easy and there are several things left, I'm asking you again to use a demo for now on which you can iterate quickly and have a short feedback loop. Instead of restarting Workbench every time you try something or make a typo. It will also be easier for me to help you.

Once we're satisfied with the demo and its features, we can move the code to CodeView

@sonnyp sonnyp self-assigned this Jul 2, 2023
@sonnyp sonnyp changed the title Adds CodeFind feature Add CodeFind feature Jul 2, 2023
@SoNiC-HeRE
Copy link
Contributor Author

I'll apply the changes and shift the code to a demo

@SoNiC-HeRE
Copy link
Contributor Author

SoNiC-HeRE commented Jul 4, 2023

Ported the codefind feature as a library entry/demo
Successfully ported the different functionalities and checked working

  • UI is not that great (since this is not a real demo and meant for only testing purposes);
  • Also the search and iterate works but is barely visible in dark mode

What else can i add/improve in the feature? @sonnyp

@sonnyp
Copy link
Contributor

sonnyp commented Jul 5, 2023

Ported the codefind feature as a library entry/demo

Great, it's much easier now to iterate on the logic and the UI.

UI is not that great (since this is not a real demo and meant for only testing purposes);

We should iterate on the UI of the search bar in the demo too.

Also the search and iterate works but is barely visible in dark mode

Works for me (light mode) :D

You can use this:

const scheme_manager = Source.StyleSchemeManager.get_default();
const style_manager = Adw.StyleManager.get_default();

function updateScheme() {
    const scheme = scheme_manager.get_scheme(
      style_manager.dark ? "Adwaita-dark" : "Adwaita",
    );
    buffer.set_style_scheme(scheme);
}

updateScheme()
style_manager.connect("notify::dark", updateScheme);

It will be useful for the Source View demo anyway.

What else can i add/improve in the feature?

The ticket says

It should behave like the one in GNOME Text Editor.

Please make it look and work the same. Including the shortcut for Esc. Please pay attention to details such as (but not only)

  • the previous/next buttons are disabled if the search entry is empty
  • each button has a tooltip with the shortcut

Ignore the buttons "Search & Replace" and "Togge Search options".

@SoNiC-HeRE
Copy link
Contributor Author

SoNiC-HeRE commented Jul 6, 2023

Implemented all the functionalities that i saw in the text editor's searchbar.

  • I need help with line 145 and line 155 in main.js as they dont work neither gives error.
    The updatelabel function works fine when i use the buttons but not using event controllers.

    Fixed in commit f4a7578

  • Also, i tried using searchcontext.get_occurrences_count function to trigger the sensitivity of buttons to false when it gives -1 value but it does'nt work.
    Fixed in commit dd3f11e

  • I also get a warning

Gtk-WARNING **: 03:05:42.951: Finalizing GtkSearchEntry 0x5566c09c42d0, but it still has children left:
   - GtkLabel 0x5566c3e62d20

but i think i should ignore it cause i used a label widget as a child of searchentry so as to make them look linked.

Everything else works fine as expected.

@SoNiC-HeRE SoNiC-HeRE requested a review from sonnyp July 6, 2023 21:32
@sonnyp sonnyp marked this pull request as ready for review July 8, 2023 20:24
Copy link
Contributor

@sonnyp sonnyp left a comment

Choose a reason for hiding this comment

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

There are a bunch of problems. Please compare UI and features with GNOME Text Editor as requested previously.

  • Ctrl+F doesn't focus the entry (can't type directly after)
  • Ctrl+F then Esc then Ctrl+F doesn't work (consider focusing the code view on ESC)
  • The spacing between the elements of the bar are different than GNOME Text Editor
  • There is no Clear Entry button in GNOME Text Editor
  • GNOME Text Editor doesn't have a placeholder
  • The Search Bar bottom corners are rounded in GNOME Text Editor
  • "Next match" on the last match should go to the first match (cycles) and vice versa
  • After hiding the search (ESC), opening the search bar should immediately enable the search
  • Closing the search should not clear the entry

Less obvious ones:

  • Enter should be a shortcut to "Move to next Match"
  • Shift+Enter should be the a shortcut to "Move to previous Match"
  • The close button should be rounded (check with hovering it with your cursor)

Let me know if something isn't clear

@SoNiC-HeRE
Copy link
Contributor Author

SoNiC-HeRE commented Jul 10, 2023

In response to #364 (review)
I've successfully implemented the following changes

  • Ctrl+F doesn't focus the entry (can't type directly after)
  • Ctrl+F then Esc then Ctrl+F doesn't work (consider focusing the code view on ESC)
  • The spacing between the elements of the bar are different than GNOME Text Editor
  • GNOME Text Editor doesn't have a placeholder
  • The Search Bar bottom corners are rounded in GNOME Text Editor
  • "Next match" on the last match should go to the first match (cycles) and vice versa
  • After hiding the search (ESC), opening the search bar should immediately enable the search
  • Closing the search should not clear the entry
  • Shift+Enter should be the a shortcut to "Move to previous Match"
  • The close button should be rounded (check with hovering it with your cursor)

Still Unsolved:

  • There is no Clear Entry button in GNOME Text Editor
    Since we are using Gtk.SearchBar here which precomes with clear entry button; i'm unsure of how to get rid of it/hide it.Also GNOME Text Editor doesn't use Gtk.SearchBar; however i think using a Gtk.SearchBar is much more useful cause of it's properties and comes with a Gtk.Revealer.
  • Enter should be a shortcut to "Move to next Match"
    I've implemented the logic for Enter key shortcut for "Move to next Match" still it doesn't seem to work;However similar code for keybinding Shift+Enter shortcut for "Move to previous Match" works smoothly.

Other:

  • I tried updating the label of search entry with total no of occurrences whenever a search is made; however the get_occurrences_count property always returns -1. Line no 239 in main.js I'm unsure why this is happening and need help with this

Please let me know the changes that are required

@SoNiC-HeRE SoNiC-HeRE requested a review from sonnyp July 10, 2023 22:47
@sonnyp sonnyp mentioned this pull request Dec 18, 2023
@sonnyp
Copy link
Contributor

sonnyp commented Jan 14, 2024

#853

@sonnyp sonnyp closed this Jan 14, 2024
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.

Implement code find
2 participants