Enhance: Property page enhancements #6024
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
logseq-cldwalker
changed the title
logseq-cldwalker
force-pushed
the
fix/remove-built-in-property-pages
branch
from
747ab69
to
bb56b63
Compare
logseq-cldwalker
changed the title
llcc
reviewed
Jul 14, 2022
logseq-cldwalker
changed the title
logseq-cldwalker
changed the title
| @@ -464,7 +475,7 @@ | |||
|
|
|||
| (defn get-page-refs-from-properties | |||
| [format properties db date-formatter] | |||
| (let [page-refs (get-page-ref-names-from-properties format properties)] | |||
| (let [page-refs (get-page-ref-names-from-properties format properties {})] | |||
There was a problem hiding this comment.
I didn't pass the user config here as I didn't understand if this config mattered here. If it does, there is some small work to find all callers to this fn outside the graph-parser and pass in (state/get-config) to it
llcc
reviewed
Jul 15, 2022
llcc
force-pushed
the
fix/remove-built-in-property-pages
branch
from
3034397
to
3254c0b
Compare
Also fix carve linter and make nbb test invocation easier
Introduced hidden naming as that is a more accurate name for its usage. There may be a possible bug with existing built-in-properties but not familiar with all cases here and don't want to introduce more bugs before release
e.g. numbers, boolean
cnrpman
force-pushed
the
fix/remove-built-in-property-pages
branch
from
3cd196b
to
44c7208
Compare
llcc
force-pushed
the
fix/remove-built-in-property-pages
branch
from
44c7208
to
df07b86
Compare
llcc
force-pushed
the
fix/remove-built-in-property-pages
branch
from
df07b86
to
cd6c355
Compare
llcc
reviewed
Jul 15, 2022
llcc
reviewed
Jul 15, 2022
There was a problem hiding this comment.
Thanks @logseq-cldwalker. It works as expected now.
I added some fixes to this PR and commented on every change.
Produced two many extra pages.
They were also being referred to elsewhere in block.cljs Also moved filters to editable as that is existing behavior Also disabled verbosity of cli tests to allow graph-parser tests to be more readable
|
Thanks for the reviews and additional fixes |
Merged
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.
As a follow up to #5922, this does the following:
This PR is an alternative PR to #6009 and #6022. I tested this by removing all page properties from
logseq/pages-metadata.ednand then re-indexing