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

core: validate yaml definitions (go templates) #8597

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ngosang
Copy link
Member

@ngosang ngosang commented May 10, 2020

DISCLAIMER!!! This is not tested and not reviewed. Maybe this PR is not necessary at all.

Take your time to review line by line and test the indexers if necessary. I can split this PR is smaller ones or rebase if it's required.

Don't block the other issues, let this for when you don't have other things to do. :)

@ngosang ngosang requested a review from garfield69 May 10, 2020 21:56
@@ -95,14 +95,14 @@ search:
- name: re_replace # S01E01 to Сезон 1 Серии 1
args: ["(?i)\\bS0*(\\d+)E0*(\\d+)\\b", "езон $1 ерии $2"]
inputs:
$raw: "{{range .Categories}}c{{.}}=1&{{end}}"
$raw: "{{ range .Categories }}c{{ . }}=1&{{ end }}"
Copy link
Contributor

Choose a reason for hiding this comment

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

the current Cardigann code will not accept any variation, returning (with slightly different keys)

Jackett.Common.IndexerException: Exception (0daykiev): The given key '. ' was not present in the dictionary.
 ---> System.Collections.Generic.KeyNotFoundException: The given key '. ' was not present in the dictionary.
   at System.Collections.Generic.Dictionary`2.get_Item(TKey key)
   at Jackett.Common.Indexers.CardigannIndexer.applyGoTemplateText(String template, Dictionary`2 variables, TemplateTextModifier modifier) in d:\a\1\s\src\Jackett.Common\Indexers\CardigannIndexer.cs:line 441
   at Jackett.Common.Indexers.CardigannIndexer.PerformQuery(TorznabQuery query) in d:\a\1\s\src\Jackett.Common\Indexers\CardigannIndexer.cs:line 1302
   at Jackett.Common.Indexers.BaseIndexer.ResultsForQuery(TorznabQuery query) in d:\a\1\s\src\Jackett.Common\Indexers\BaseIndexer.cs:line 323
   --- End of inner exception stack trace ---
   at Jackett.Common.Indexers.BaseIndexer.ResultsForQuery(TorznabQuery query) in d:\a\1\s\src\Jackett.Common\Indexers\BaseIndexer.cs:line 343
   at Jackett.Common.Indexers.BaseWebIndexer.ResultsForQuery(TorznabQuery query) in d:\a\1\s\src\Jackett.Common\Indexers\BaseIndexer.cs:line 799
   at Jackett.Server.Controllers.ResultsController.Results(ApiSearch requestt) in d:\a\1\s\src\Jackett.Server\Controllers\ResultsController.cs:line 224

for any of the following

$raw: "{{ range .Categories }}c{{ . }}=1&{{ end }}"
$raw: "{{ range .Categories }}c{{. }}=1&{{ end }}"
$raw: "{{ range .Categories }}c{{ .}}=1&{{ end }}"
$raw: "{{ range .Categories }}c{{ . }}=1&{{end}}"
$raw: "{{ range .Categories }}c{{ .}}=1&{{end}}"
$raw: "{{ range .Categories }}c{{. }}=1&{{end}}"
$raw: "{{ range .Categories }}c{{.}}=1&{{ end }}"
$raw: "{{ range .Categories }}c{{.}}=1&{{end }}"

the late variation returned The given key '.}}=1&{{end' was not present in the dictionary.

only

`$raw: "{{ range .Categories }}c{{.}}=1&{{end}}"`

is accepted.

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you prefer the current implementation c{{.}}=1&{{end}} or with spaces c{{ . }}=1&{{ end }}?
I don't care, you are in charge or this part. I can make the changes in Cardigann if required.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems to me that the current implementation of the range .Categories is out of step with what Cardigann does with the other templates, so yes, if you can amend Cardigann to handle spaces when processing the range then that is a worth doing. Thanks.

@ngosang ngosang self-assigned this Sep 15, 2020
@ngosang ngosang added this to In Progress in Jackett Core Dec 21, 2020
@bakerboy448
Copy link
Contributor

Close given 507f2f2?

@garfield69
Copy link
Contributor

garfield69 commented Aug 14, 2022

Close?

I don't think so. The range statement has a hardcoded dependence on {{end}} which is out of step with the other statements that allow spaces on either side of the end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Jackett Core
In progress
Development

Successfully merging this pull request may close these issues.

None yet

3 participants