Feature: Consisent zk-link-format #63
Merged
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.
|
Introduces a small potential breaking change, for those who customize zk-link-format. But otherwise, seems alright. Thanks |
|
Hey, turns out this breaks |
boyechko
added a commit
to boyechko/octavo
that referenced
this pull request
Jul 15, 2023
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).
Good catch. Fixed. |
boyechko
added a commit
to boyechko/octavo
that referenced
this pull request
Jul 15, 2023
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).
boyechko
added a commit
to boyechko/octavo
that referenced
this pull request
Jul 15, 2023
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 changes
zk-link-formatto use same %-sequences as otherzk-*-formatvariables.Having consistent syntax is helpful to avoid mistakes. Additionally, this change allows 1) simplifying
zk--formatso 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.Also,
zk-link-regexpfunction 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.