meow-edit / meow

Yet another modal editing on Emacs / 猫态编辑
GNU General Public License v3.0
1.07k stars 128 forks source link

Entering regex into `meow-visit` with `meow-visit-sanitize-completion` enabled does not work #431

Closed akho closed 1 year ago

akho commented 1 year ago

When meow-visit-sanitize-completion is enabled, regexps entered in meow-visit minibuffer prompt are sanitized, so you cannot actually enter a regexp, only search for symbols. This is because meow--prompt-symbol-and-words uses regexp-quote for its output when meow-visit-sanitize-completion is enabled.

I think the intention was just to have a cleaner view of completion candidates, not to prevent the user from entering regexes.

Removing the second check and the quoting seems to work for me, but the behavior was introduced in c29239b with message “bugfix for meow-visit”; I’m reluctant to submit a pull request without knowing what the bug was.

This is what seems to work for me:

(defun meow--prompt-symbol-and-words (prompt beg end)
  "Completion with PROMPT for symbols and words from BEG to END."
  (let ((completions))
    (save-mark-and-excursion
      (goto-char beg)
      (while (re-search-forward "\\_<\\(\\sw\\|\\s_\\)+\\_>" end t)
        (let ((result (match-string-no-properties 0)))
          (when (>= (length result) meow-visit-collect-min-length)
            (if meow-visit-sanitize-completion
                (push (cons result (format "\\_<%s\\_>" (regexp-quote result))) completions)
              (push (format "\\_<%s\\_>" (regexp-quote result)) completions))))))
    (setq completions (delete-dups completions))
    (let ((selected (completing-read prompt completions nil nil)))
       (or (cdr (assoc selected completions))
           selected))))

Edit: mentioning @shitianshu as the author of the original commit.

DogLooksGood commented 1 year ago

That's me, but damned, I guess I forget the reason. I think it's an issue when you trying to enter a raw non-regexp search. Because the default candidates are sanitized, and people may not realize it's a regexp. So when people do a raw search, for example, foo.bar, the behavior doesn't make sense to user.

akho commented 1 year ago

But the result is that you either see ugly completion options, or lose regexp search (…in meow-friendly bindings), and the docs are incorrect. Maybe it would make sense to set two options, for completion candidates and for raw/regexp?

Though I don’t think anyone actually prefers unsanitized completion candidates, and raw/regexp should really be different commands. The nature of meow-visit is more fitting for raw search, as symbol completion is mostly useless for regexp.

(on a side note, it would be really nice to have incremental search as a motion, with selection, number hints, and working in beacon state)

DogLooksGood commented 1 year ago

I think the default behavior now is you either select one candidate, or search for raw string. The regexp quote will prevent it to be treated as regexp. What's your proposal? Select from the candidate or enter a regexp? I'm good with introducing an option for this behavior, as long as you can distinguish the raw input.

The number hints are for expanding, so having it with search is inconsistent. The idea of meow is not stick with its own search functionality(even though it is for now), but integrate with existing search-ring or regexp-search-ring. However, all packages use their own variables in their own ways, makes it really difficult to support.

akho commented 1 year ago

I think the default behavior now is you either select one candidate, or search for raw string. The regexp quote will prevent it to be treated as regexp. What's your proposal? Select from the candidate or enter a regexp? I'm good with introducing an option for this behavior, as long as you can distinguish the raw input.

I think that the current meow-visit needs a change in the docstring: it currently says that it reads a regexp from the minibuffer, but that’s confusing, as you can’t actually give it a regexp.

Regexp search should be a separate command, I think, without completion to words/symbols, but with incremental search and highlighting as-you-type. I guess existing Emacs facilities are fine for me.

The number hints are for expanding, so having it with search is inconsistent. The idea of meow is not stick with its own search functionality(even though it is for now), but integrate with existing search-ring or regexp-search-ring. However, all packages use their own variables in their own ways, makes it really difficult to support.

I’ll open a separate issue for this separate discussion :)

akho commented 1 year ago

I think we can close this — meow-visit doc is in line with behavior, and regexp search is out of scope.