terminal icon indicating copy to clipboard operation
terminal copied to clipboard

#11: Findbar arrow sensitivity

Open hollyschilling opened this issue 3 years ago • 8 comments

Added changes to the sensitivity of next and previous buttons when no results are found.

hollyschilling avatar Sep 18 '22 15:09 hollyschilling

@jeremypw What linter and config is being used?

hollyschilling avatar Sep 23 '22 13:09 hollyschilling

@hollyschilling The linter is vala-lint with the default configuration. You can download and install locally from https://github.com/vala-lang/vala-lint. You can then check you code locally before committing.

jeremypw avatar Sep 23 '22 18:09 jeremypw

Further info on elementary code style, which the linter enforces can be found here: https://docs.elementary.io/develop/writing-apps/code-style

There should probably be info about linting locally there but as yet it has not been added.

jeremypw avatar Sep 23 '22 18:09 jeremypw

I'm sorry to keep bugging you for simple questions, but I'm having an error with the linting that I can't understand. Specifically, I received and error stating:

  117.26    error   Expected variable name in underscore_convention   naming-convention

The referenced line of code is bool initialFound = next_search ();. I can't understand why this declaration would be preceded with an underscore. For context, the line is in a try block, in a lambda, in the constructor. What is the rule that is triggering this to get that error?

hollyschilling avatar Sep 23 '22 23:09 hollyschilling

@hollyschilling in Vala variables are named using snake_case, and initialFound should be initial_found

lenemter avatar Sep 24 '22 05:09 lenemter

A side-effect of this change is that if, after the search, the terminal produces output containing the search term then the navigation buttons remain insensitive even if the cyclic search button is toggled. The search term has to be modified or re-entered in order to initiate a new search. A more convenient way of refreshing the search would be good. At least if the cyclic search button is toggled then the sensitivity of the navigation buttons should be revisited.

jeremypw avatar Sep 25 '22 11:09 jeremypw

I completely agree that this is potentially undesirable behavior. Is there an event emitted from the terminal that would indicate that additional content has been added?

There are a few such edge cases that are not handled well. I feel that adding messaging to indicate that the end of search has been reached would be appropriate.

hollyschilling avatar Sep 25 '22 14:09 hollyschilling

Is there an event emitted from the terminal that would indicate that additional content has been added?

There is Vte.Terminal.content-changed signal. But you may need to throttle that so that a large number of searches are not performed unnecessarily when the content is changing rapidly. Maybe better leave that for another PR.

jeremypw avatar Sep 25 '22 17:09 jeremypw