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

Support for all built-in Jekyll filters: slugify, number_of_words, date_to_xmlschema, date_to_rfc822 #443

Closed
Nowaker opened this issue Jan 19, 2022 · 15 comments

Comments

@Nowaker
Copy link
Contributor

Nowaker commented Jan 19, 2022

liquidjs is "missing" a couple Jekyll-specific filters:

  • slugify
  • number_of_words
  • date_to_xmlschema
  • date_to_rfc822

https://jekyllrb.com/docs/liquid/filters

Would you be willing to include them as part of Liquidjs? If so, I'll create a PR. Please let me know.

@harttle
Copy link
Owner

harttle commented Jan 20, 2022

It'll be great to have these filters! Except for Jekyll logic specific ones like relative_url.

1 similar comment
@harttle
Copy link
Owner

harttle commented Jan 20, 2022

It'll be great to have these filters! Except for Jekyll logic specific ones like relative_url.

@Nowaker Nowaker changed the title Support for all built-in Jekyll filters: slugify, number_of_words Support for all built-in Jekyll filters: slugify, number_of_words, date_to_xmlschema, date_to_rfc822 Feb 5, 2022
@TomasHubelbauer
Copy link
Contributor

TomasHubelbauer commented May 22, 2023

I'm specifically missing the push filter to add an element to an array. Liquid's append seems to be adding a string to my array character-wise. @harttle such new filter would be added to https://github.com/harttle/liquidjs/blob/master/src/filters/array.ts right? And it could look something like this? (Inspired by concat)

export function push<T> (v: T[], arg: T): T[] {
  assert(arguments.length === 2, 'push expects 2 arguments')
  v = toValue(v)
  arg = toValue(arg)
  return toArray(v).concat([arg])
}

Or even just:

export function push<T> (v: T[], arg: T): T[] {
  return concat(v, [arg])
}

I added this in #611. I hope you like the implementation and decide to accept the PR. Once the PR is merged, does the Playground automatically update to support it?

@TomasHubelbauer
Copy link
Contributor

TomasHubelbauer commented May 22, 2023

Ideas for how to support the other filters. I used a full checkbox where I found the filter already in the codebase but did not check too deeply if it is really it or how it is implemented.

Update (by @harttle): will use the following list to track latest status.

  • relative_url - probably no reasonable way to support?
    • yes, we don't have a page concept.
  • absolute_url - just append to https://example.com/? Not super meaningful but could be a difference between someone being able to use LiquidJS Playground out of the box or not if they didn't care for the URL but their code was using this filter.
    • Sorry I don't get the point. Non-exist filters will be just ignored and it also works in playground. I think it's kinda same to have a wrong prefix with no prefix.
  • date_to_xmlschema - I think this is just new Date(toValue(v)).toISOString()?
  • date_to_rfc822 - I think this is just new Date(toValue(v)).toUTCString()?
  • date_to_string - similar to the above but custom formatting
  • date_to_string: "ordinal", "US" - ditto
  • date_to_long_string - ditto
  • date_to_long_string: "ordinal" - ditto
  • where - implement using filter as this is for arrays only - already done
  • where_exp - this could be filter + eval for simplicity I think or is there a mechanism for expression parsing?
  • find - array find
  • find_exp - again find + eval unless there is an expression parser already
  • group_by - reduce
  • group_by_exp - reduce + eval
  • xml_escape - not sure what exactly this does but probably some/full overlap with encodeURIComponent
  • cgi_escape - or maybe this is encodeURIComponent?
  • uri_escape - or this? :D
  • number_of_words - this one is gonna be complex and I don't have any experience with handling Asian scripts but for the start I could add a simple latin oriented filter which splits by a space and returns the length of that
  • array_to_sentence_string - looks like a simple join with an extra condition
  • markdownify - probably can lean on some popular MD NPM library here - I made Add support for the markdownify filter #620 + feat: add support for the Jekyll markdownify filter #621
    • As markdown is quite a lot of work, and can be very different for frontend projects, we better let users pick the engine rather than integrate within liquidjs, especially for the UMD build. I want to keep liquid as a tool instead of a framework (picking other tools). But I'm not against having a doc to describe how to add this support when using liquidjs.
  • smartify - this might be just a replace?
  • sassify - again a candidate for an NPM package
    • we don't include other npm packages, so this won't be implemented inside liquidjs. But again, we can create a guide or another repo to maintain these filters to be used with LiquidJS
  • slugify - should be possible with a regex assuming the rules of the slugification are well known
  • jsonify- just JSON.stringify - this could potentially be an alias for the existing json filter?
  • normalize_whitespace - regex
  • sort - array sort - already done
  • sample - array index access + Math.random - I made feat: Add support for the Jekyll sample filter #612
  • to_integer - Number constructor
  • push, pop, shift, unshift - the JS array method - I made feat: Add support for the Jekyll push filter #611 + test: Add tests for push #614
  • pop - the JS array method
  • shift - the JS array method
  • unshift - the JS array method
  • inspect - I think we can just use JSON.stringify here and we don't need to follow the same format as Jekyll but the one possible edge case is if someone used the output of this in capture

I am also interested in include_relative and it would be cool to have the Playground have some UI to allow you to upload a file for this and process it client-side to make this work.

@harttle
Copy link
Owner

harttle commented May 23, 2023

I added this in #611. I hope you like the implementation and decide to accept the PR.

As long as Shopify/liquid implements this, I'm good to also add this feature.

Once the PR is merged, does the Playground automatically update to support it?

Yes, it's on CI build for master branch.

@TomasHubelbauer
Copy link
Contributor

According to this link it is one of the built-in Jekyll filters: https://jekyllrb.com/docs/liquid/filters. I took this issue to mean there is interest in adding Jekyll-specific filters on top of the base Liquid language. Are you willing to still accept this filter in that case?

@TomasHubelbauer
Copy link
Contributor

@harttle I don't see a CI build here: https://github.com/harttle/liquidjs/actions
Did you mean the GitHub Pages deployment step? Or the Check workflow?

@harttle
Copy link
Owner

harttle commented May 23, 2023

@TomasHubelbauer sorry I thought it was brought from shopify.

But it's also OK to bring jekyll filters to LiquidJS as long as it's not jekyll specific (I mean relying on Jekyll config or anything only available in Jekyll, or any concept only makes sense for Jekyll sites).

I don't see a CI build here

There's a docs flow https://github.com/harttle/liquidjs/blob/master/.github/workflows/docs.yml

@TomasHubelbauer
Copy link
Contributor

Then I regret to inform you that I think #611 might not be working. :( I am trying the snippet in the comments there in the Playground to no avail…

@harttle
Copy link
Owner

harttle commented May 23, 2023

No worry, there's a lot work remaining anyway. Thanks for your effort.

@TomasHubelbauer
Copy link
Contributor

@harttle I added tests for push in #614 and turns out that implementation is fine, the reason it doesn't work on the Playground must be something else. Locally I had a problem running build:docs:

+ ./bin/build-contributors.sh
sed: -e: No such file or directory
sed: 1: "docs/themes/navy/layout ...": extra characters at the end of d command

But on CI there is no such issue: https://github.com/harttle/liquidjs/actions/runs/5060635227/jobs/9083770703

So I don't think this is why push is not working on the Playground. Do you have any ideas on why else it might be?

@bzerangue
Copy link

It'll be great to have these filters! Except for Jekyll logic specific ones like relative_url.

@harttle - Any chance that the group_by filter can be added?

github-actions bot pushed a commit that referenced this issue Apr 14, 2024
# [10.11.0](v10.10.2...v10.11.0) (2024-04-14)

### Features

* group_by/group_by_exp/find/find_exp from Jekyll, [#443](#443) ([2b713b7](2b713b7))
* pop/shift/unshift filters from Jekyll ([258780e](258780e))
@harttle
Copy link
Owner

harttle commented Apr 14, 2024

Added group_by, group_by_exp, find, find_exp in the latest version, @bzerangue please try v10.11.0

@bzerangue
Copy link

bzerangue commented Apr 16, 2024 via email

harttle added a commit that referenced this issue May 13, 2024
github-actions bot pushed a commit that referenced this issue May 13, 2024
# [10.13.0](v10.12.0...v10.13.0) (2024-05-13)

### Features

* array_to_sentence_string and number_of_words filters from Jekyll, [#443](#443) ([50253a9](50253a9))
* date filters from Jekyll ([4955e75](4955e75))
* escape filters from Jekyll, [#443](#443) ([b12eb8a](b12eb8a))
* jsonify, inspect, to_integer, normalize_whitespace filters ([842b45c](842b45c))
* slugify filter from Jekyll, [#443](#443) ([47ddc11](47ddc11))
@harttle
Copy link
Owner

harttle commented May 13, 2024

As we've tried to introduce all Jekyll filters listed on this issue and explained each of those can't be introduced, I'll close this issue.

For further/missing filters not covered by this issue, feel free to file specific requests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants