unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* "search --path=directory/" is lame(-ish)
@ 2014-10-29 17:07 David Edmondson
  2014-10-29 17:14 ` Tomi Ollila
                   ` (3 more replies)
  0 siblings, 4 replies; 16+ messages in thread
From: David Edmondson @ 2014-10-29 17:07 UTC (permalink / raw)
  To: notmuch

Adding a terminal slash to a directory name when using --path causes the
search to fail. Removing the terminal slash produces results.

Given that many shells will add the terminal slash during completion,
this is lame(-ish).

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: "search --path=directory/" is lame(-ish)
  2014-10-29 17:07 "search --path=directory/" is lame(-ish) David Edmondson
@ 2014-10-29 17:14 ` Tomi Ollila
  2014-10-29 19:43 ` Jani Nikula
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 16+ messages in thread
From: Tomi Ollila @ 2014-10-29 17:14 UTC (permalink / raw)
  To: David Edmondson, notmuch

On Wed, Oct 29 2014, David Edmondson <dme@dme.org> wrote:

> Adding a terminal slash to a directory name when using --path causes the
> search to fail. Removing the terminal slash produces results.
>
> Given that many shells will add the terminal slash during completion,
> this is lame(-ish).

Except zsh(1) which adds it, but when pressing enter to complete command
execution, the trailing slash is removed -- at least in my configuration...

... Nevertheless, I agree that the "feature" we have is at least a bit lame ;D

Tomi

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: "search --path=directory/" is lame(-ish)
  2014-10-29 17:07 "search --path=directory/" is lame(-ish) David Edmondson
  2014-10-29 17:14 ` Tomi Ollila
@ 2014-10-29 19:43 ` Jani Nikula
  2014-10-30  8:21   ` David Edmondson
  2017-03-29 10:34 ` David Bremner
  2022-01-27 12:04 ` "search --path=directory/" is lame(-ish) David Bremner
  3 siblings, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2014-10-29 19:43 UTC (permalink / raw)
  To: David Edmondson, notmuch

On Wed, 29 Oct 2014, David Edmondson <dme@dme.org> wrote:
> Adding a terminal slash to a directory name when using --path causes the
> search to fail. Removing the terminal slash produces results.

I think you mean path:, not --path. Anyway, the reason for this
behaviour is that the path components are indexed as boolean terms, not
unlike tags, just with a different namespace. It's all parsed in Xapian,
not in Notmuch. Adding the / variants would mean indexing twice the
amount of terms.

This could be fixed with our own query parser (somewhere at the other
end of the rainbow), but for the time being I don't see a reasonable
fix.

> Given that many shells will add the terminal slash during completion,
> this is lame(-ish).

Given that path: expects a relative path from the maildir root, not just
any path, and the notmuch bash completion script (if you happen to use
bash) does exactly this, without adding the slash, I'm not too worried.

None of this should be taken as disagreeing with you, though! ;)


BR,
Jani.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: "search --path=directory/" is lame(-ish)
  2014-10-29 19:43 ` Jani Nikula
@ 2014-10-30  8:21   ` David Edmondson
  2014-10-31  7:36     ` David Bremner
  0 siblings, 1 reply; 16+ messages in thread
From: David Edmondson @ 2014-10-30  8:21 UTC (permalink / raw)
  To: Jani Nikula, notmuch

On Wed, Oct 29 2014, Jani Nikula wrote:
> On Wed, 29 Oct 2014, David Edmondson <dme@dme.org> wrote:
>> Adding a terminal slash to a directory name when using --path causes the
>> search to fail. Removing the terminal slash produces results.
>
> I think you mean path:, not --path.

Yes, sorry.

> Anyway, the reason for this behaviour is that the path components are
> indexed as boolean terms, not unlike tags, just with a different
> namespace. It's all parsed in Xapian, not in Notmuch. Adding the /
> variants would mean indexing twice the amount of terms.

Could we always prune a trailing slash from the path: component of a
query before using it?

>> Given that many shells will add the terminal slash during completion,
>> this is lame(-ish).
>
> Given that path: expects a relative path from the maildir root, not just
> any path, and the notmuch bash completion script (if you happen to use
> bash) does exactly this, without adding the slash, I'm not too worried.

I'm almost always doing this in Emacs shell-mode, manipulating the
pathnames on the fly. This means that I can adapt, of course.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: "search --path=directory/" is lame(-ish)
  2014-10-30  8:21   ` David Edmondson
@ 2014-10-31  7:36     ` David Bremner
  0 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2014-10-31  7:36 UTC (permalink / raw)
  To: David Edmondson, notmuch

David Edmondson <dme@dme.org> writes:

>
> Could we always prune a trailing slash from the path: component of a
> query before using it?
>

As I understand it, this is complicated by the fact that we pass the
whole string to Xapian to be parsed as a query, so we don't really know
where the path: terms are. We could in principle preprocess the string
(more) but that seems to be pretty fragile, and we'd have to minimally
deal with quoting of e.g. paths with spaces in them.

d

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: "search --path=directory/" is lame(-ish)
  2014-10-29 17:07 "search --path=directory/" is lame(-ish) David Edmondson
  2014-10-29 17:14 ` Tomi Ollila
  2014-10-29 19:43 ` Jani Nikula
@ 2017-03-29 10:34 ` David Bremner
  2022-01-21 23:38   ` strip trailing '/' from path/folder query David Bremner
  2022-01-27 12:04 ` "search --path=directory/" is lame(-ish) David Bremner
  3 siblings, 1 reply; 16+ messages in thread
From: David Bremner @ 2017-03-29 10:34 UTC (permalink / raw)
  To: David Edmondson, notmuch

David Edmondson <dme@dme.org> writes:

> Adding a terminal slash to a directory name when using --path causes the
> search to fail. Removing the terminal slash produces results.
>
> Given that many shells will add the terminal slash during completion,
> this is lame(-ish).

This would be relatively straightforward to impliment on top of

     id:20170324121436.28978-2-david@tethera.net

In particular add a filter to strip trailing / in the non-regex case.

d

^ permalink raw reply	[flat|nested] 16+ messages in thread

* strip trailing '/' from path/folder query.
  2017-03-29 10:34 ` David Bremner
@ 2022-01-21 23:38   ` David Bremner
  2022-01-21 23:38     ` [PATCH 1/6] test: sanitize generated message files names David Bremner
                       ` (7 more replies)
  0 siblings, 8 replies; 16+ messages in thread
From: David Bremner @ 2022-01-21 23:38 UTC (permalink / raw)
  To: David Bremner, David Edmondson, notmuch

It would not be that much more work to strip any number of '/'
(instead of just 1) for infix queries, but this change already covers
>99% of the issues with path completion.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH 1/6] test: sanitize generated message files names
  2022-01-21 23:38   ` strip trailing '/' from path/folder query David Bremner
@ 2022-01-21 23:38     ` David Bremner
  2022-01-21 23:38     ` [PATCH 2/6] test: known broken tests for trailing / in path search (infix) David Bremner
                       ` (6 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2022-01-21 23:38 UTC (permalink / raw)
  To: David Bremner, David Edmondson, notmuch

It is fragile to encode the generated names into tests, as it makes
tests break when e.g. new tests are added.  There is a possibility
that this will hide certain failures; in that case meaningful filenames
should be chosen for the generated messages.
---
 test/T081-sexpr-search.sh            |  2 +-
 test/T100-search-by-folder.sh        |  6 +++---
 test/T370-search-folder-coherence.sh |  4 ++--
 test/T750-gzip.sh                    | 14 +++++++-------
 test/test-lib.sh                     |  2 +-
 5 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/test/T081-sexpr-search.sh b/test/T081-sexpr-search.sh
index 73f45041..0e5af635 100755
--- a/test/T081-sexpr-search.sh
+++ b/test/T081-sexpr-search.sh
@@ -346,7 +346,7 @@ output=$(notmuch search --query=sexp '(attachment (starts-with not))' | notmuch_
 test_expect_equal "$output" 'thread:XXX   2009-11-18 [2/2] Lars Kellogg-Stedman; [notmuch] "notmuch help" outputs to stderr? (attachment inbox signed unread)'
 
 test_begin_subtest "starts-with, folder"
-notmuch search --output=files --query=sexp '(folder (starts-with bad))' | notmuch_dir_sanitize | sed 's/[0-9]*$/XXX/' > OUTPUT
+notmuch search --output=files --query=sexp '(folder (starts-with bad))' | notmuch_search_files_sanitize > OUTPUT
 cat <<EOF > EXPECTED
 MAIL_DIR/bad/msg-XXX
 MAIL_DIR/bad/news/msg-XXX
diff --git a/test/T100-search-by-folder.sh b/test/T100-search-by-folder.sh
index 409cfdcc..fc9ad5f8 100755
--- a/test/T100-search-by-folder.sh
+++ b/test/T100-search-by-folder.sh
@@ -28,8 +28,8 @@ test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1(2)] Notmuch Test Suite
 
 test_begin_subtest "Folder search with --output=files"
 output=$(notmuch search --output=files folder:bad/news | notmuch_search_files_sanitize)
-test_expect_equal "$output" "MAIL_DIR/bad/news/msg-003
-MAIL_DIR/duplicate/bad/news/msg-003"
+test_expect_equal "$output" "MAIL_DIR/bad/news/msg-XXX
+MAIL_DIR/duplicate/bad/news/msg-XXX"
 
 test_begin_subtest "After removing duplicate instance of matching path"
 rm -r "${MAIL_DIR}/bad/news"
@@ -39,7 +39,7 @@ test_expect_equal "$output" ""
 
 test_begin_subtest "Folder search with --output=files part #2"
 output=$(notmuch search --output=files folder:duplicate/bad/news | notmuch_search_files_sanitize)
-test_expect_equal "$output" "MAIL_DIR/duplicate/bad/news/msg-003"
+test_expect_equal "$output" "MAIL_DIR/duplicate/bad/news/msg-XXX"
 
 test_begin_subtest "After removing duplicate instance of matching path part #2"
 output=$(notmuch search folder:duplicate/bad/news | notmuch_search_sanitize)
diff --git a/test/T370-search-folder-coherence.sh b/test/T370-search-folder-coherence.sh
index 0a2727e7..cf202bb3 100755
--- a/test/T370-search-folder-coherence.sh
+++ b/test/T370-search-folder-coherence.sh
@@ -24,8 +24,8 @@ test_expect_equal "$output" "No new mail."
 
 test_begin_subtest "Multiple files for same message"
 cat <<EOF >EXPECTED
-MAIL_DIR/msg-001
-MAIL_DIR/spam/msg-001
+MAIL_DIR/msg-XXX
+MAIL_DIR/spam/msg-XXX
 EOF
 notmuch search --output=files id:$id_x | notmuch_search_files_sanitize >OUTPUT
 test_expect_equal_file EXPECTED OUTPUT
diff --git a/test/T750-gzip.sh b/test/T750-gzip.sh
index 4408d085..5648896f 100755
--- a/test/T750-gzip.sh
+++ b/test/T750-gzip.sh
@@ -58,13 +58,13 @@ test_expect_equal_file EXPECTED OUTPUT
 test_begin_subtest "notmuch search --output=files with partially gzipped mail store"
 notmuch search --output=files '*' | notmuch_search_files_sanitize > OUTPUT
 cat <<EOF > EXPECTED
-MAIL_DIR/msg-001.gz
-MAIL_DIR/msg-002.gz
-MAIL_DIR/msg-003.gz
-MAIL_DIR/msg-004
-MAIL_DIR/msg-005.gz
-MAIL_DIR/msg-006
-MAIL_DIR/msg-007.gz
+MAIL_DIR/msg-XXX.gz
+MAIL_DIR/msg-XXX.gz
+MAIL_DIR/msg-XXX.gz
+MAIL_DIR/msg-XXX
+MAIL_DIR/msg-XXX.gz
+MAIL_DIR/msg-XXX
+MAIL_DIR/msg-XXX.gz
 EOF
 test_expect_equal_file EXPECTED OUTPUT
 
diff --git a/test/test-lib.sh b/test/test-lib.sh
index f1275b85..02b67f02 100644
--- a/test/test-lib.sh
+++ b/test/test-lib.sh
@@ -537,7 +537,7 @@ notmuch_search_sanitize () {
 }
 
 notmuch_search_files_sanitize () {
-    notmuch_dir_sanitize
+    notmuch_dir_sanitize |  sed 's/msg-[0-9][0-9][0-9]/msg-XXX/'
 }
 
 notmuch_dir_sanitize () {
-- 
2.34.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 2/6] test: known broken tests for trailing / in path search (infix)
  2022-01-21 23:38   ` strip trailing '/' from path/folder query David Bremner
  2022-01-21 23:38     ` [PATCH 1/6] test: sanitize generated message files names David Bremner
@ 2022-01-21 23:38     ` David Bremner
  2022-01-21 23:38     ` [PATCH 3/6] test/sexp: tests for path, folder, including trailing '/' (sexp) David Bremner
                       ` (5 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2022-01-21 23:38 UTC (permalink / raw)
  To: David Bremner, David Edmondson, notmuch

In [1], David Edmondson observed that the trailing / added by many
completion mechanisms causes path searches to silently fail.  This
test reproduces that bug for both `path:' and `folder:' searches.

[1]: id:cunoasuolcv.fsf@gargravarr.hh.sledj.net
---
 test/T100-search-by-folder.sh | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/test/T100-search-by-folder.sh b/test/T100-search-by-folder.sh
index fc9ad5f8..fbc68266 100755
--- a/test/T100-search-by-folder.sh
+++ b/test/T100-search-by-folder.sh
@@ -31,6 +31,12 @@ output=$(notmuch search --output=files folder:bad/news | notmuch_search_files_sa
 test_expect_equal "$output" "MAIL_DIR/bad/news/msg-XXX
 MAIL_DIR/duplicate/bad/news/msg-XXX"
 
+test_begin_subtest "Folder search with --output=files (trailing /)"
+test_subtest_known_broken
+output=$(notmuch search --output=files folder:bad/news/ | notmuch_search_files_sanitize)
+test_expect_equal "$output" "MAIL_DIR/bad/news/msg-XXX
+MAIL_DIR/duplicate/bad/news/msg-XXX"
+
 test_begin_subtest "After removing duplicate instance of matching path"
 rm -r "${MAIL_DIR}/bad/news"
 notmuch new
@@ -120,6 +126,14 @@ test_expect_equal "$output" "MAIL_DIR/bar/17:2,
 MAIL_DIR/bar/18:2,
 MAIL_DIR/cur/51:2,"
 
+test_begin_subtest "path: search (trailing /)"
+test_subtest_known_broken
+output=$(notmuch search --output=files path:"bar/" | notmuch_search_files_sanitize | sort)
+# cur/51:2, is a duplicate of bar/18:2,
+test_expect_equal "$output" "MAIL_DIR/bar/17:2,
+MAIL_DIR/bar/18:2,
+MAIL_DIR/cur/51:2,"
+
 test_begin_subtest "top level path: search"
 output=$(notmuch search --output=files path:'""' | notmuch_search_files_sanitize | sort)
 test_expect_equal "$output" "MAIL_DIR/01:2,
-- 
2.34.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 3/6] test/sexp: tests for path, folder, including trailing '/' (sexp)
  2022-01-21 23:38   ` strip trailing '/' from path/folder query David Bremner
  2022-01-21 23:38     ` [PATCH 1/6] test: sanitize generated message files names David Bremner
  2022-01-21 23:38     ` [PATCH 2/6] test: known broken tests for trailing / in path search (infix) David Bremner
@ 2022-01-21 23:38     ` David Bremner
  2022-01-21 23:38     ` [PATCH 4/6] lib: drop trailing slash for path and folder searches (infix) David Bremner
                       ` (4 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2022-01-21 23:38 UTC (permalink / raw)
  To: David Bremner, David Edmondson, notmuch

This duplicates the bug reported in [1], as well as adding some simple
regression tests for 'path' and 'folder' searches which were
previously missing for sexp syntax.

[1]: id:cunoasuolcv.fsf@gargravarr.hh.sledj.net
---
 test/T081-sexpr-search.sh | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/test/T081-sexpr-search.sh b/test/T081-sexpr-search.sh
index 0e5af635..a355f8b6 100755
--- a/test/T081-sexpr-search.sh
+++ b/test/T081-sexpr-search.sh
@@ -185,6 +185,28 @@ notmuch search folder:'""' > EXPECTED
 notmuch search --query=sexp '(folder "")'  > OUTPUT
 test_expect_equal_file EXPECTED OUTPUT
 
+test_begin_subtest "Search by 'folder' with --output=files"
+output=$(notmuch search --output=files --query=sexp '(folder bad/news)' | notmuch_search_files_sanitize)
+test_expect_equal "$output" "MAIL_DIR/bad/news/msg-XXX
+MAIL_DIR/duplicate/bad/news/msg-XXX"
+
+test_begin_subtest "Search by 'folder' with --output=files (trailing /)"
+test_subtest_known_broken
+output=$(notmuch search --output=files --query=sexp '(folder bad/news/)' | notmuch_search_files_sanitize)
+test_expect_equal "$output" "MAIL_DIR/bad/news/msg-XXX
+MAIL_DIR/duplicate/bad/news/msg-XXX"
+
+test_begin_subtest "Search by 'path' with --output=files"
+output=$(notmuch search --output=files --query=sexp '(path bad/news)' | notmuch_search_files_sanitize)
+test_expect_equal "$output" "MAIL_DIR/bad/news/msg-XXX
+MAIL_DIR/duplicate/bad/news/msg-XXX"
+
+test_begin_subtest "Search by 'path' with --output=files (trailing /)"
+test_subtest_known_broken
+output=$(notmuch search --output=files --query=sexp '(path bad/news/)' | notmuch_search_files_sanitize)
+test_expect_equal "$output" "MAIL_DIR/bad/news/msg-XXX
+MAIL_DIR/duplicate/bad/news/msg-XXX"
+
 test_begin_subtest "Search by 'id'"
 add_message '[subject]="search by id"' '[date]="Sat, 01 Jan 2000 12:00:00 -0000"'
 output=$(notmuch search --query=sexp "(id ${gen_msg_id})" | notmuch_search_sanitize)
-- 
2.34.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 4/6] lib: drop trailing slash for path and folder searches (infix)
  2022-01-21 23:38   ` strip trailing '/' from path/folder query David Bremner
                       ` (2 preceding siblings ...)
  2022-01-21 23:38     ` [PATCH 3/6] test/sexp: tests for path, folder, including trailing '/' (sexp) David Bremner
@ 2022-01-21 23:38     ` David Bremner
  2022-01-21 23:38     ` [PATCH 5/6] test: add multiple path, folder sexp query tests David Bremner
                       ` (3 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2022-01-21 23:38 UTC (permalink / raw)
  To: David Bremner, David Edmondson, notmuch

This resolves an old bug reported by David Edmondson in 2014. The fix
is only needed for the "boolean" case, as probabilistic / phrase
searching already ignores punctuation.

This fix is only for the infix (xapian provided) query parser.

[1]: id:cunoasuolcv.fsf@gargravarr.hh.sledj.net
---
 lib/database-private.h        |  1 +
 lib/prefix.cc                 |  4 ++--
 lib/regexp-fields.cc          | 10 +++++++++-
 test/T100-search-by-folder.sh |  2 --
 4 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/lib/database-private.h b/lib/database-private.h
index 0c08fa15..657b1aa1 100644
--- a/lib/database-private.h
+++ b/lib/database-private.h
@@ -165,6 +165,7 @@ typedef enum {
     NOTMUCH_FIELD_EXTERNAL	= 1 << 0,
     NOTMUCH_FIELD_PROBABILISTIC = 1 << 1,
     NOTMUCH_FIELD_PROCESSOR	= 1 << 2,
+    NOTMUCH_FIELD_STRIP_TRAILING_SLASH	= 1 << 3,
 } notmuch_field_flag_t;
 
 /*
diff --git a/lib/prefix.cc b/lib/prefix.cc
index 0d92bdd7..857c05b9 100644
--- a/lib/prefix.cc
+++ b/lib/prefix.cc
@@ -46,7 +46,7 @@ prefix_t prefix_table[] = {
     { "mid",                    "Q",            NOTMUCH_FIELD_EXTERNAL |
       NOTMUCH_FIELD_PROCESSOR },
     { "path",                   "P",            NOTMUCH_FIELD_EXTERNAL |
-      NOTMUCH_FIELD_PROCESSOR },
+      NOTMUCH_FIELD_PROCESSOR | NOTMUCH_FIELD_STRIP_TRAILING_SLASH },
     { "property",               "XPROPERTY",    NOTMUCH_FIELD_EXTERNAL },
     /*
      * Unconditionally add ':' to reduce potential ambiguity with
@@ -55,7 +55,7 @@ prefix_t prefix_table[] = {
      * discussion.
      */
     { "folder",                 "XFOLDER:",     NOTMUCH_FIELD_EXTERNAL |
-      NOTMUCH_FIELD_PROCESSOR },
+      NOTMUCH_FIELD_PROCESSOR | NOTMUCH_FIELD_STRIP_TRAILING_SLASH },
     { "date",                   NULL,           NOTMUCH_FIELD_EXTERNAL |
       NOTMUCH_FIELD_PROCESSOR },
     { "query",                  NULL,           NOTMUCH_FIELD_EXTERNAL |
diff --git a/lib/regexp-fields.cc b/lib/regexp-fields.cc
index c6d9d94f..7e9d959c 100644
--- a/lib/regexp-fields.cc
+++ b/lib/regexp-fields.cc
@@ -235,7 +235,15 @@ RegexpFieldProcessor::operator() (const std::string & str)
 	    return parser.parse_query (query_str, NOTMUCH_QUERY_PARSER_FLAGS, term_prefix);
 	} else {
 	    /* Boolean prefix */
-	    std::string term = term_prefix + str;
+	    std::string query_str;
+	    std::string term;
+
+	    if (str.length () > 1 && str.at (str.size () - 1) == '/')
+		query_str = str.substr (0, str.size () - 1);
+	    else
+		query_str = str;
+
+	    term = term_prefix + query_str;
 	    return Xapian::Query (term);
 	}
     }
diff --git a/test/T100-search-by-folder.sh b/test/T100-search-by-folder.sh
index fbc68266..0123d0b2 100755
--- a/test/T100-search-by-folder.sh
+++ b/test/T100-search-by-folder.sh
@@ -32,7 +32,6 @@ test_expect_equal "$output" "MAIL_DIR/bad/news/msg-XXX
 MAIL_DIR/duplicate/bad/news/msg-XXX"
 
 test_begin_subtest "Folder search with --output=files (trailing /)"
-test_subtest_known_broken
 output=$(notmuch search --output=files folder:bad/news/ | notmuch_search_files_sanitize)
 test_expect_equal "$output" "MAIL_DIR/bad/news/msg-XXX
 MAIL_DIR/duplicate/bad/news/msg-XXX"
@@ -127,7 +126,6 @@ MAIL_DIR/bar/18:2,
 MAIL_DIR/cur/51:2,"
 
 test_begin_subtest "path: search (trailing /)"
-test_subtest_known_broken
 output=$(notmuch search --output=files path:"bar/" | notmuch_search_files_sanitize | sort)
 # cur/51:2, is a duplicate of bar/18:2,
 test_expect_equal "$output" "MAIL_DIR/bar/17:2,
-- 
2.34.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 5/6] test: add multiple path, folder sexp query tests
  2022-01-21 23:38   ` strip trailing '/' from path/folder query David Bremner
                       ` (3 preceding siblings ...)
  2022-01-21 23:38     ` [PATCH 4/6] lib: drop trailing slash for path and folder searches (infix) David Bremner
@ 2022-01-21 23:38     ` David Bremner
  2022-01-21 23:38     ` [PATCH 6/6] lib: strip trailing '/' from pathnames (sexp queries) David Bremner
                       ` (2 subsequent siblings)
  7 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2022-01-21 23:38 UTC (permalink / raw)
  To: David Bremner, David Edmondson, notmuch

This is mainly to make sure we get trailing / removal correct. Also
add regression test for path: in the infix parser matching the
existing one for folder:.
---
 test/T081-sexpr-search.sh     | 26 ++++++++++++++++++++++++++
 test/T100-search-by-folder.sh |  6 ++++++
 2 files changed, 32 insertions(+)

diff --git a/test/T081-sexpr-search.sh b/test/T081-sexpr-search.sh
index a355f8b6..ebf2c59f 100755
--- a/test/T081-sexpr-search.sh
+++ b/test/T081-sexpr-search.sh
@@ -196,6 +196,19 @@ output=$(notmuch search --output=files --query=sexp '(folder bad/news/)' | notmu
 test_expect_equal "$output" "MAIL_DIR/bad/news/msg-XXX
 MAIL_DIR/duplicate/bad/news/msg-XXX"
 
+test_begin_subtest "Search by 'folder' (multiple)"
+output=$(notmuch search --query=sexp '(folder bad bad/news things/bad)' | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; To the bone (inbox unread)
+thread:XXX   2001-01-05 [1/1(2)] Notmuch Test Suite; Bears (inbox unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Bites, stings, sad feelings (inbox unread)"
+
+test_begin_subtest "Search by 'folder' (multiple, trailing /)"
+test_subtest_known_broken
+output=$(notmuch search --query=sexp '(folder bad bad/news/ things/bad)' | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; To the bone (inbox unread)
+thread:XXX   2001-01-05 [1/1(2)] Notmuch Test Suite; Bears (inbox unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Bites, stings, sad feelings (inbox unread)"
+
 test_begin_subtest "Search by 'path' with --output=files"
 output=$(notmuch search --output=files --query=sexp '(path bad/news)' | notmuch_search_files_sanitize)
 test_expect_equal "$output" "MAIL_DIR/bad/news/msg-XXX
@@ -207,6 +220,19 @@ output=$(notmuch search --output=files --query=sexp '(path bad/news/)' | notmuch
 test_expect_equal "$output" "MAIL_DIR/bad/news/msg-XXX
 MAIL_DIR/duplicate/bad/news/msg-XXX"
 
+test_begin_subtest "Search by 'path' specification (multiple)"
+output=$(notmuch search --query=sexp '(path bad bad/news things/bad)' | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; To the bone (inbox unread)
+thread:XXX   2001-01-05 [1/1(2)] Notmuch Test Suite; Bears (inbox unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Bites, stings, sad feelings (inbox unread)"
+
+test_begin_subtest "Search by 'path' specification (multiple, trailing /)"
+test_subtest_known_broken
+output=$(notmuch search --query=sexp '(path bad bad/news/ things/bad)' | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; To the bone (inbox unread)
+thread:XXX   2001-01-05 [1/1(2)] Notmuch Test Suite; Bears (inbox unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Bites, stings, sad feelings (inbox unread)"
+
 test_begin_subtest "Search by 'id'"
 add_message '[subject]="search by id"' '[date]="Sat, 01 Jan 2000 12:00:00 -0000"'
 output=$(notmuch search --query=sexp "(id ${gen_msg_id})" | notmuch_search_sanitize)
diff --git a/test/T100-search-by-folder.sh b/test/T100-search-by-folder.sh
index 0123d0b2..b4f6294e 100755
--- a/test/T100-search-by-folder.sh
+++ b/test/T100-search-by-folder.sh
@@ -18,6 +18,12 @@ test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; T
 thread:XXX   2001-01-05 [1/1(2)] Notmuch Test Suite; Bears (inbox unread)
 thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Bites, stings, sad feelings (inbox unread)"
 
+test_begin_subtest "search by path: specification (multiple)"
+output=$(notmuch search path:bad path:bad/news path:things/bad | notmuch_search_sanitize)
+test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; To the bone (inbox unread)
+thread:XXX   2001-01-05 [1/1(2)] Notmuch Test Suite; Bears (inbox unread)
+thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Bites, stings, sad feelings (inbox unread)"
+
 test_begin_subtest "Top level folder"
 output=$(notmuch search folder:'""' | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Top level (inbox unread)"
-- 
2.34.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH 6/6] lib: strip trailing '/' from pathnames (sexp queries).
  2022-01-21 23:38   ` strip trailing '/' from path/folder query David Bremner
                       ` (4 preceding siblings ...)
  2022-01-21 23:38     ` [PATCH 5/6] test: add multiple path, folder sexp query tests David Bremner
@ 2022-01-21 23:38     ` David Bremner
  2022-01-21 23:42     ` strip trailing '/' from path/folder query David Bremner
  2022-01-27 11:59     ` David Bremner
  7 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2022-01-21 23:38 UTC (permalink / raw)
  To: David Bremner, David Edmondson, notmuch

This changes makes the sexp query parser consistent with the infix one
in ignoring trailing '/'. Here we do a bit better and ignore any
number of trailing '/'.
---
 lib/parse-sexp.cc         | 14 +++++++++++---
 test/T081-sexpr-search.sh |  4 ----
 2 files changed, 11 insertions(+), 7 deletions(-)

diff --git a/lib/parse-sexp.cc b/lib/parse-sexp.cc
index 06825dc4..08fd7037 100644
--- a/lib/parse-sexp.cc
+++ b/lib/parse-sexp.cc
@@ -33,6 +33,7 @@ typedef enum {
     SEXP_FLAG_DO_EXPAND = 1 << 7,
     SEXP_FLAG_ORPHAN	= 1 << 8,
     SEXP_FLAG_RANGE	= 1 << 9,
+    SEXP_FLAG_PATHNAME	= 1 << 10,
 } _sexp_flag_t;
 
 /*
@@ -72,7 +73,8 @@ static _sexp_prefix_t prefixes[] =
     { "from",           Xapian::Query::OP_AND,          Xapian::Query::MatchAll,
       SEXP_FLAG_FIELD | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND },
     { "folder",         Xapian::Query::OP_OR,           Xapian::Query::MatchNothing,
-      SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND },
+      SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND |
+      SEXP_FLAG_PATHNAME },
     { "id",             Xapian::Query::OP_OR,           Xapian::Query::MatchNothing,
       SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX },
     { "infix",          Xapian::Query::OP_INVALID,      Xapian::Query::MatchAll,
@@ -94,7 +96,8 @@ static _sexp_prefix_t prefixes[] =
     { "or",             Xapian::Query::OP_OR,           Xapian::Query::MatchNothing,
       SEXP_FLAG_NONE },
     { "path",           Xapian::Query::OP_OR,           Xapian::Query::MatchNothing,
-      SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX },
+      SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX |
+      SEXP_FLAG_PATHNAME },
     { "property",       Xapian::Query::OP_AND,          Xapian::Query::MatchAll,
       SEXP_FLAG_FIELD | SEXP_FLAG_BOOLEAN | SEXP_FLAG_WILDCARD | SEXP_FLAG_REGEX | SEXP_FLAG_EXPAND },
     { "query",          Xapian::Query::OP_INVALID,      Xapian::Query::MatchNothing,
@@ -545,8 +548,13 @@ _sexp_to_xapian_query (notmuch_database_t *notmuch, const _sexp_prefix_t *parent
 	    return _sexp_parse_wildcard (notmuch, parent, env, "", output);
 	}
 
+	char *atom = sx->val;
+
+	if (parent && parent->flags & SEXP_FLAG_PATHNAME)
+	    strip_trailing (atom, '/');
+
 	if (parent && (parent->flags & SEXP_FLAG_BOOLEAN)) {
-	    output = Xapian::Query (term_prefix + sx->val);
+	    output = Xapian::Query (term_prefix + atom);
 	    return NOTMUCH_STATUS_SUCCESS;
 	}
 
diff --git a/test/T081-sexpr-search.sh b/test/T081-sexpr-search.sh
index ebf2c59f..e2936cd7 100755
--- a/test/T081-sexpr-search.sh
+++ b/test/T081-sexpr-search.sh
@@ -191,7 +191,6 @@ test_expect_equal "$output" "MAIL_DIR/bad/news/msg-XXX
 MAIL_DIR/duplicate/bad/news/msg-XXX"
 
 test_begin_subtest "Search by 'folder' with --output=files (trailing /)"
-test_subtest_known_broken
 output=$(notmuch search --output=files --query=sexp '(folder bad/news/)' | notmuch_search_files_sanitize)
 test_expect_equal "$output" "MAIL_DIR/bad/news/msg-XXX
 MAIL_DIR/duplicate/bad/news/msg-XXX"
@@ -203,7 +202,6 @@ thread:XXX   2001-01-05 [1/1(2)] Notmuch Test Suite; Bears (inbox unread)
 thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Bites, stings, sad feelings (inbox unread)"
 
 test_begin_subtest "Search by 'folder' (multiple, trailing /)"
-test_subtest_known_broken
 output=$(notmuch search --query=sexp '(folder bad bad/news/ things/bad)' | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; To the bone (inbox unread)
 thread:XXX   2001-01-05 [1/1(2)] Notmuch Test Suite; Bears (inbox unread)
@@ -215,7 +213,6 @@ test_expect_equal "$output" "MAIL_DIR/bad/news/msg-XXX
 MAIL_DIR/duplicate/bad/news/msg-XXX"
 
 test_begin_subtest "Search by 'path' with --output=files (trailing /)"
-test_subtest_known_broken
 output=$(notmuch search --output=files --query=sexp '(path bad/news/)' | notmuch_search_files_sanitize)
 test_expect_equal "$output" "MAIL_DIR/bad/news/msg-XXX
 MAIL_DIR/duplicate/bad/news/msg-XXX"
@@ -227,7 +224,6 @@ thread:XXX   2001-01-05 [1/1(2)] Notmuch Test Suite; Bears (inbox unread)
 thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; Bites, stings, sad feelings (inbox unread)"
 
 test_begin_subtest "Search by 'path' specification (multiple, trailing /)"
-test_subtest_known_broken
 output=$(notmuch search --query=sexp '(path bad bad/news/ things/bad)' | notmuch_search_sanitize)
 test_expect_equal "$output" "thread:XXX   2001-01-05 [1/1] Notmuch Test Suite; To the bone (inbox unread)
 thread:XXX   2001-01-05 [1/1(2)] Notmuch Test Suite; Bears (inbox unread)
-- 
2.34.1

^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: strip trailing '/' from path/folder query.
  2022-01-21 23:38   ` strip trailing '/' from path/folder query David Bremner
                       ` (5 preceding siblings ...)
  2022-01-21 23:38     ` [PATCH 6/6] lib: strip trailing '/' from pathnames (sexp queries) David Bremner
@ 2022-01-21 23:42     ` David Bremner
  2022-01-27 11:59     ` David Bremner
  7 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2022-01-21 23:42 UTC (permalink / raw)
  To: David Edmondson, notmuch

David Bremner <david@tethera.net> writes:

> It would not be that much more work to strip any number of '/'
> (instead of just 1) for infix queries, but this change already covers
>>99% of the issues with path completion.

Apologies, I knew there was something else I meant to say here, but
blanked. This series is intended to be applied on top of [1]. The order
could be changed, but they are both adding a flag to the sexp parser.

[1]: id:20220120133605.31401-2-david@tethera.net

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: strip trailing '/' from path/folder query.
  2022-01-21 23:38   ` strip trailing '/' from path/folder query David Bremner
                       ` (6 preceding siblings ...)
  2022-01-21 23:42     ` strip trailing '/' from path/folder query David Bremner
@ 2022-01-27 11:59     ` David Bremner
  7 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2022-01-27 11:59 UTC (permalink / raw)
  To: David Edmondson, notmuch

David Bremner <david@tethera.net> writes:

> It would not be that much more work to strip any number of '/'
> (instead of just 1) for infix queries, but this change already covers
>>99% of the issues with path completion.

I have applied this series to master.

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: "search --path=directory/" is lame(-ish)
  2014-10-29 17:07 "search --path=directory/" is lame(-ish) David Edmondson
                   ` (2 preceding siblings ...)
  2017-03-29 10:34 ` David Bremner
@ 2022-01-27 12:04 ` David Bremner
  3 siblings, 0 replies; 16+ messages in thread
From: David Bremner @ 2022-01-27 12:04 UTC (permalink / raw)
  To: David Edmondson, notmuch

David Edmondson <dme@dme.org> writes:

> Adding a terminal slash to a directory name when using --path causes the
> search to fail. Removing the terminal slash produces results.
>
> Given that many shells will add the terminal slash during completion,
> this is lame(-ish).

This bug should be fixed as of 2c1d1107f5dacdb4a2c514909fd96f45f83e2f3c

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2022-01-27 12:04 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-10-29 17:07 "search --path=directory/" is lame(-ish) David Edmondson
2014-10-29 17:14 ` Tomi Ollila
2014-10-29 19:43 ` Jani Nikula
2014-10-30  8:21   ` David Edmondson
2014-10-31  7:36     ` David Bremner
2017-03-29 10:34 ` David Bremner
2022-01-21 23:38   ` strip trailing '/' from path/folder query David Bremner
2022-01-21 23:38     ` [PATCH 1/6] test: sanitize generated message files names David Bremner
2022-01-21 23:38     ` [PATCH 2/6] test: known broken tests for trailing / in path search (infix) David Bremner
2022-01-21 23:38     ` [PATCH 3/6] test/sexp: tests for path, folder, including trailing '/' (sexp) David Bremner
2022-01-21 23:38     ` [PATCH 4/6] lib: drop trailing slash for path and folder searches (infix) David Bremner
2022-01-21 23:38     ` [PATCH 5/6] test: add multiple path, folder sexp query tests David Bremner
2022-01-21 23:38     ` [PATCH 6/6] lib: strip trailing '/' from pathnames (sexp queries) David Bremner
2022-01-21 23:42     ` strip trailing '/' from path/folder query David Bremner
2022-01-27 11:59     ` David Bremner
2022-01-27 12:04 ` "search --path=directory/" is lame(-ish) David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).