-
Notifications
You must be signed in to change notification settings - Fork 13
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
Feature: consistent zk-link-format and POSIX regexp conversion for grep #68
Open
boyechko
wants to merge
6
commits into
localauthor:main
Choose a base branch
from
boyechko:feature/consisent-zk-link-format-and-posix-regexp
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Feature: consistent zk-link-format and POSIX regexp conversion for grep #68
boyechko
wants to merge
6
commits into
localauthor:main
from
boyechko:feature/consisent-zk-link-format-and-posix-regexp
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* `zk-format-function`'s docstring pointed to a very sparse `zk--format` docstring. * Other `*-format` variables had wrong information (`zk-format-function` should never be nil) and repeated information that pertains properly only to `zk-format-id-and-title`.
It's generally a good practice to avoid hardcoded values, but this also allows the user to customize what the title looks like.
Since `zk--grep-file-list` expects a regexp anyway, might as well use the `zk-link-regexp` to generate it rather than doing it locally and repeating code.
This increases consistency, which should help to avoid obscure bugs.
Having consistent syntax is helpful to avoid mistakes. Additionally, this change allows 1) simplifying `zk--format` so it doesn't need to handle two different sets of %-sequences, and 2) experimenting with other link markup, such as `[[%i][%t]]` for org-mode links without explicit link type or `[[%i|%t]]` for syntax used by MediaWiki.
So far, functions like `zk--grep-file-list` have just passed Elisp-style regexp to `grep` and hoped it works, which it does for zk-ID or other literal strings. Changes to `zk-link-regexp` in PR localauthor#63 broke things because it introduced explicit capture groups (`\(?1:...\)`), which is not supported by POSIX regexps that `grep` uses. The new function, `zk--posix-regexp`, does some basic conversion from Elisp-style regexps to POSIX-style. There is package `pcre2el` that does it in a more complete and sophisticated way, but for our purposes, this should be enough. Functions that pass regexps to `grep`, such as `zk--grep-file-list`, `zk--grep-tag-list`, and `zk--grep-link-id-list` are updated to sanitize ("POSIXize") the regexps they pass. The cost is minuscule ("Elapsed time: 0.000379s" for 10,000 calls with a complex regexp).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a new version of PR #63 that fixes the
zk-backlinks
bug.Consistent zk-link-format
This changes
zk-link-format
to use same %-sequences as otherzk-*-format
variables.Having consistent syntax is helpful to avoid mistakes. Additionally, this change allows 1) simplifying
zk--format
so it doesn't need to handle two different sets of %-sequences, and 2) experimenting with other link markup, such as[[%i][%t]]
for org-mode links without explicit link type or[[%i|%t]]
for syntax used by MediaWiki.Extend zk-link-regexp to generate targeted regexps
Also,
zk-link-regexp
function now accepts optional ID and/or title to generate a regexp matching specific links, so that with the defaultzk-link-format
,(zk-link-regexp "202307090142")
returns"\\[\\[\\(?1:202307090142\\)]]"
. The explicitly numbered capture groups are consistent withzk-file-name-regexp
.Introduce zk--posix-regexp
So far, functions like
zk--grep-file-list
have just passed Elisp-style regexp togrep
and hoped it works, which it does for zk-ID or other literal strings. Changes tozk-link-regexp
in PR #63 broke things because it introduced explicit capture groups (\(?1:...\)
), which is not supported by POSIX regexps thatgrep
uses.The new function,
zk--posix-regexp
, does some basic conversion from Elisp-style regexps to POSIX-style. There is packagepcre2el
that does it in a more complete and sophisticated way, but for our purposes, this should be enough.Functions that pass regexps to
grep
, such aszk--grep-file-list
,zk--grep-tag-list
, andzk--grep-link-id-list
are updated to sanitize ("POSIXize") the regexps they pass. The cost is minuscule ("Elapsed time: 0.000379s" for 10,000 calls with a complex regexp).