Re: Make "wrap" the default in find for 2.4.0?

2021-06-01 Thread Joel Kulesza
On Tue, Jun 1, 2021 at 10:27 PM Scott Kostyshak  wrote:

> I've been using the new find bar in master and it's great! Thank you
> Jürgen for the nice feature.
>
> Is there any interest in enabling the "wrap" checkbox for find as default?
> I have no idea what most users prefer. I could make a poll on the lyx-users
> list if there is interest in considering it.
>

One question on polls and because the ones hosted via email seem
potentially inconclusive and subject to distracting discussion: is there
merit in hosting the poll using a polling service (Google Forms, Quick
Survey [https://github.com/simonv3/quick-survey/] at lyx.org, etc.)?  In
this way, I wonder if discussion could be separated from results.


> I'm not sure how relevant the following comparison is given that LyX is
> different, but in case it is helpful for discussion:
>
> LibreOffice Writer, Gedit, Chromium, and Firefox automatically wrap. I'm
> not sure any of them even has an option to not wrap or to prompt. The only
> application I'm aware of that has wrap as an option is Vim (see config
> "wrapscan"), although the default is to wrap.
>

As a vim user, this capability would feel familiar, but I have no strong
preference one way or the other.  However, vim keybindings in LyX would be
a (substantially) different matter.

Thanks,
Joel
-- 
lyx-devel mailing list
lyx-devel@lists.lyx.org
http://lists.lyx.org/mailman/listinfo/lyx-devel


Make "wrap" the default in find for 2.4.0?

2021-06-01 Thread Scott Kostyshak
I've been using the new find bar in master and it's great! Thank you Jürgen for 
the nice feature.

Is there any interest in enabling the "wrap" checkbox for find as default? I 
have no idea what most users prefer. I could make a poll on the lyx-users list 
if there is interest in considering it.

I'm not sure how relevant the following comparison is given that LyX is 
different, but in case it is helpful for discussion:

LibreOffice Writer, Gedit, Chromium, and Firefox automatically wrap. I'm not 
sure any of them even has an option to not wrap or to prompt. The only 
application I'm aware of that has wrap as an option is Vim (see config 
"wrapscan"), although the default is to wrap.

I attach a small set of patches related to find. In addition to setting wrap as 
default, the patches introduce a status message when there is auto-wrap.

Scott
From a12dd7639fbf8fa86f51004cff7a3684d90724a0 Mon Sep 17 00:00:00 2001
From: Scott Kostyshak 
Date: Tue, 1 Jun 2021 23:59:40 -0400
Subject: [PATCH 1/3] refactor: keep meaning of a variable consistent

We now use a new variable, "wrap", to track if a wrap should be
done, which is true either if "auto_wrap" is true or if the user
chooses to wrap in the dialog.

This preserves the meaning of the "auto_wrap" variable and also
removes the confusion of why the previous code of

  if (!auto_wrap) {
...
  }
  if (auto_wrap) {

did not use an "else" instead of the second "if".

No change in functionality.
---
 src/lyxfind.cpp | 7 ---
 src/lyxfind.h   | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/src/lyxfind.cpp b/src/lyxfind.cpp
index 444db69a98..820274e89c 100644
--- a/src/lyxfind.cpp
+++ b/src/lyxfind.cpp
@@ -276,7 +276,7 @@ bool searchAllowed(docstring const & str)
 
 bool findOne(BufferView * bv, docstring const & searchstr,
 	 bool case_sens, bool whole, bool forward,
-	 bool find_del, bool check_wrap, bool auto_wrap,
+	 bool find_del, bool check_wrap, bool const auto_wrap,
 	 bool instant, bool onlysel)
 {
 	// Clean up previous selections with empty searchstr on instant
@@ -338,6 +338,7 @@ bool findOne(BufferView * bv, docstring const & searchstr,
 	}
 	else if (check_wrap) {
 		DocIterator cur_orig(bv->cursor());
+		bool wrap = auto_wrap;
 		if (!auto_wrap) {
 			docstring q;
 			if (forward)
@@ -348,9 +349,9 @@ bool findOne(BufferView * bv, docstring const & searchstr,
   "Continue searching from the end?");
 			int wrap_answer = frontend::Alert::prompt(_("Wrap search?"),
 q, 0, 1, _("&Yes"), _("&No"));
-			auto_wrap = wrap_answer == 0;
+			wrap = wrap_answer == 0;
 		}
-		if (auto_wrap) {
+		if (wrap) {
 			if (forward) {
 bv->cursor().clear();
 bv->cursor().push_back(CursorSlice(bv->buffer().inset()));
diff --git a/src/lyxfind.h b/src/lyxfind.h
index f79ef240cd..b700e0ce1a 100644
--- a/src/lyxfind.h
+++ b/src/lyxfind.h
@@ -74,7 +74,7 @@ bool lyxfind(BufferView * bv, FuncRequest const & ev);
 bool findOne(BufferView * bv, docstring const & searchstr,
 	 bool case_sens, bool whole, bool forward,
 	 bool find_del = true, bool check_wrap = false,
-	 bool auto_wrap = false, bool instant = false,
+	 bool const auto_wrap = false, bool instant = false,
 	 bool onlysel = false);
 
 /** Parse the string encoding of the replace request that is found in
-- 
2.20.1

From 0ac7f74348d5cfdad377ef6634dd06284ae1a104 Mon Sep 17 00:00:00 2001
From: Scott Kostyshak 
Date: Tue, 1 Jun 2021 23:59:50 -0400
Subject: [PATCH 2/3] find: give status message when auto-wrapping

When auto-wrap is enabled, it is sometimes informative to know
when the search wraps.

A status message is consistent with LibreOffice and Vim.
---
 src/lyxfind.cpp | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/lyxfind.cpp b/src/lyxfind.cpp
index 820274e89c..e9e1d0e016 100644
--- a/src/lyxfind.cpp
+++ b/src/lyxfind.cpp
@@ -359,6 +359,12 @@ bool findOne(BufferView * bv, docstring const & searchstr,
 bv->cursor().setCursor(doc_iterator_end(&bv->buffer()));
 bv->cursor().backwardPos();
 			}
+			if (auto_wrap) {
+docstring const msg = forward
+  ? _("Search reached end of document, continuing from beginning.")
+  : _("Search reached beginning of document, continuing from end.");
+bv->message(msg);
+			}
 			bv->clearSelection();
 			if (findOne(bv, searchstr, case_sens, whole, forward,
 find_del, false, false, false, false))
-- 
2.20.1

From e5c3505d17b1026f9f446c4e5be9cadf0eb09736 Mon Sep 17 00:00:00 2001
From: Scott Kostyshak 
Date: Tue, 1 Jun 2021 23:59:57 -0400
Subject: [PATCH 3/3] find: auto-wrap by default

---
 src/frontends/qt/ui/SearchUi.ui | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/frontends/qt/ui/SearchUi.ui b/src/frontends/qt/ui/SearchUi.ui
index ed6a4c8e10..1837d6ed2a 100644
--- a/src/frontends/qt/ui/SearchUi.ui
+++ b/src/frontends/qt/ui/SearchUi.ui
@@ -246,6 +246,9 @@
   
&Wrap
   
+  
+   true
+