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

Start Resources :counter first time they're used #4335

Closed
kaushalmodi opened this Issue Jan 26, 2018 · 4 comments

Comments

Projects
None yet
2 participants
@kaushalmodi
Copy link
Member

kaushalmodi commented Jan 26, 2018

Hello,

This is a follow-up issue which proposes a fix for the issue in #4334.

Here's a copy of the proposal from the discussion on discourse for convenience:


In the example I provided, there are 3 PDF files, and I wanted to first set the title of just one pdf file.

So the alternative you provided:

The counter is grouped by the matcher + first occurrence of either title or name.
If you had said:

[[resources]]
src = “documents/photo_specs.pdf”
[resources.params]
someParam = “foo”

[[resources]]
src = "**.pdf"
name = "pdf-file-:counter"
title = “Photo Specifications #:counter”

They would be in the same group.

would not work, as that would auto-set the titles of all 3 PDF files.

I thought that my original example:

[[resources]]
  src = "documents/photo_specs.pdf"
  title = "Photo Specifications"
[[resources]]
  src = "**.pdf"
  name = "pdf-file-:counter"

was unambiguous.. set title for just one file and counter-based name for all files.

If you have suggestions for improvement to the above, they are welcome, but those should come in form of a spec that takes any ambiguity into account.

How about:

"An individual counter should be grouped to matcher for each title and name, but only for the entities that use the :counter placeholder."

So in the example I posted, the counter will be grouped only with the second matcher, as only that uses :counter.

But if the example had been:

[[resources]]
  # using arbitrary glob
  src = "**s.pdf"  
  title = "PDF s #:counter"
[[resources]]
  src = "**.pdf"
  name = "pdf-file-:counter"

"documents/photo_specs.pdf" would get a title like "PDF s #1", but name like "pdf-file-3". This was quite an arbitrary example, and thus an arbitrary outcome.

But this would then allow a normal example like in my original post to work exactly as expected.

@bep bep changed the title Proposal to create counter<>matcher association only for fields that have the ":counter" placeholder Start Resources :counter first time they're used Jan 27, 2018

@bep bep added the Enhancement label Jan 27, 2018

@bep bep self-assigned this Jan 27, 2018

@bep bep added this to the v0.35 milestone Jan 27, 2018

@bep bep closed this in 7b472e4 Jan 27, 2018

@kaushalmodi

This comment has been minimized.

Copy link
Member Author

kaushalmodi commented Jan 27, 2018

Thank you for taking care of this quickly! I confirm the fix; the exact same test in #4334 now works!

@kaushalmodi

This comment has been minimized.

Copy link
Member Author

kaushalmodi commented Jan 29, 2018

The older test in #4334 worked fine. But right now, I was trying to create a test case using the params used in 7b472e4 with 4 resource files (photo_specs.pdf, other_specs.pdf, guide.pdf, checklist.pdf), and I am finding strange results.

Here is the test case:

[[resources]]
  src = "*specs.pdf"
  title = "Specifications #:counter"
[[resources]]
  src = "**.pdf"
  name = "pdf-file-:counter"

It looks like the name and title counters are not yet independent..

  • Both checklist.pdf and other_specs.pdf are getting the Name: pdf-file-1, and
  • Both guide.pdf and photo_specs.pdf are getting the Name: pdf-file-2.

It's as if the matcher with title caused the name-counter to reset?

Do I need to create a new issue for this? Or can you reopen this?


Update (2018/01/29): The same test case now shows the correct results after the fix in df20b05. Thank you!

@bep

This comment has been minimized.

Copy link
Member

bep commented Jan 29, 2018

It's as if the matcher with title caused the name-counter to reset?

Yes, this is by design. The counter is an item counter, but maybe we could loosen up on that restriction.

But in general, I'm not too keen on discussions in closed issues. I almost never read notifications on those. But don't start a new for this, I will think about it.

bep added a commit that referenced this issue Jan 29, 2018

resource: Make resource counters for name and title independent
This is the most flexible with the current syntax, and probably what most people would expcect.

Updates #4335
@kaushalmodi

This comment has been minimized.

Copy link
Member Author

kaushalmodi commented Jan 29, 2018

Thanks for fixing this! The same test now shows the intended results. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.