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

Improve autocomplete search for pages #685

Merged
merged 4 commits into from
Nov 15, 2023
Merged

Improve autocomplete search for pages #685

merged 4 commits into from
Nov 15, 2023

Conversation

annda
Copy link
Contributor

@annda annda commented Nov 13, 2023

Fixes #579 and #593

@annda
Copy link
Contributor Author

annda commented Nov 13, 2023

Also addresses #622 by adding regex capability in new filter config.

Fixes #579 special characters in ids
Implements #593 and #622 flexibility in namespaces via regex
@annda annda marked this pull request as ready for review November 14, 2023 18:45
@annda annda requested a review from splitbrain November 15, 2023 08:17
Copy link
Member

@splitbrain splitbrain left a comment

Choose a reason for hiding this comment

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

Currently the regex only is applied to the namespace, not the whole ID. I guess that makes sense from the name of the configuration (namespace). More useful IMHO would be a match against the whole ID? Maybe we should think about how we could rename the config without losing existing configuration.

I am also not convinced that we need to differentiate between regex and non-regex configs. Any valid previous configuration should still be a valid regex, so why not always treat the config as regex and add the delimiters ourselves?

@annda
Copy link
Contributor Author

annda commented Nov 15, 2023

Currently the regex only is applied to the namespace, not the whole ID. I guess that makes sense from the name of the configuration (namespace). More useful IMHO would be a match against the whole ID? Maybe we should think about how we could rename the config without losing existing configuration.

The suggestion makes a lot of sense. I tried to introduce a new option filter and, for backwards compatibility, fill it with the old namespace in the background. But it's difficult to migrate field configs this way, since they explicitly throw out any unknown keys in

protected function mergeConfig($current, &$config)

Do you have an idea how to migrate a config option another way?

@splitbrain
Copy link
Member

Hmm maybe overwrite mergeConfig() for the Page type?

@annda
Copy link
Contributor Author

annda commented Nov 15, 2023

I converted namespace and postfix to filter. Can you take another look?

@annda annda requested a review from splitbrain November 15, 2023 15:54
Copy link
Member

@splitbrain splitbrain left a comment

Choose a reason for hiding this comment

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

LGTM

@annda annda merged commit e015685 into master Nov 15, 2023
@annda annda deleted the autocomplete branch November 15, 2023 16:25
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.

Bug? Page autocompletion aborts after hyphen (special character)
2 participants