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

Issue #26 #27

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Issue #26 #27

wants to merge 2 commits into from

Conversation

m0gg
Copy link

@m0gg m0gg commented Dec 9, 2014

rewriting manifest, deletes digest-files and entries, adds non-digest entries
cherry-picked 1588a04

concerning issue #26

Remember that this variant uses File.read/gsub/File.write to modify
 manifest*.json files
@m0gg m0gg closed this Dec 9, 2014
@m0gg m0gg reopened this Dec 9, 2014
@alexspeller
Copy link
Owner

Not so sure about this - I definitely don't think removing the digest files should be the default behaviour. I haven't looked into the manifest very much so I don't know if it makes sense to have both digest and non digest in the manifest, but I'm a bit hesitant to merge without knowing more about how it works and making removing the digested files an option rather than the default.

@m0gg
Copy link
Author

m0gg commented Dec 9, 2014

It makes no sense if you're only using the logical paths in your app. The manifest is simply a hash with the logical path as key, wich would be the same for non-digest and digest files. But if you're using hardcoded paths you'd be better off not altering the manifest. In this case the logical path would be translated to the digest file.

@@ -25,6 +25,11 @@ class Manifest
def compile_with_non_digest *args
compile_without_non_digest *args

assets_path = ::Rails.root.join('public', 'assets', 'manifest*.json')
Copy link

Choose a reason for hiding this comment

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

Can this line be replaced with @filename. The initializer for the Manifest class allows for passing a completely different manifest file name.

https://github.com/sstephenson/sprockets/blob/master/lib/sprockets/manifest.rb#L18-L54

@m0gg
Copy link
Author

m0gg commented Dec 9, 2014

@armanm initializing a Manifest with the filename was the first thing i tried. Unfortunately i had do discover that the save for Manifest class is protected. As the original intention (for my purpose) was to move/rename the digested-files it was sufficient to jsut replace the digest-filenames in the Manifest.

@m0gg
Copy link
Author

m0gg commented Dec 9, 2014

@alexspeller and i'm sorry for the wrong information, but the keys in the manifest are the filenames. An entry would like that:
"header_login_bg-c6d29c89b35f0bcc2d092d0b0f8698bc.png":{"logical_path":"header_login_bg.png","mtime":"2014-11-07T23:17:10+01:00","size":143,"digest":"c6d29c89b35f0bcc2d092d0b0f8698bc"}

But as the asset-path-helpers search for the logical path, i guess there will always be returned the first match. So you'd either need additional helper-methods for non-digest asset-paths or address them without helpers, but that would kill the logical-path <=> filename concept.

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

Successfully merging this pull request may close these issues.

3 participants