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

Implemented per-directory metadata support #190

Merged
merged 7 commits into from
Jan 23, 2014
Merged

Implemented per-directory metadata support #190

merged 7 commits into from
Jan 23, 2014

Conversation

krsch
Copy link
Contributor

@krsch krsch commented Oct 12, 2013

Implemented per-directory metadata file as it was mentioned in #152.
Format is closed to the one described in the issue, but after --- I have a glob pattern terminated by newline (space at the end are considered a part of the pattern). The path in this pattern is relative to the directory with the metadata file.
Metadata in old locations (embedded and *.metadata) override per-directory metadata and the one in a subdirectory overrides the one in a parent directory.


--------------------------------------------------------------------------------
-- | Load directory-wise metadata
loadGlobalMetadata :: Provider -> FilePath -> IO (M.Map String String)
Copy link
Owner

Choose a reason for hiding this comment

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

Maybe use Metadata instead of M.Map String String here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@jaspervdj
Copy link
Owner

Thanks, the code looks good!

However, one issue is that it doesn't seem to take invalidating into account: e.g., when we change a metadata field in

foo/metadata:

--- bar/qux.markdown
titile: A Wonderful Qux

We should rebuild foo/bar/qux.markdown. This might be a little tricky to implement though. I'll have a look at it as well.

Additionally, we also want to add a test case in tests/.

@krsch
Copy link
Contributor Author

krsch commented Oct 15, 2013

I will try to add new step to Hakyll.Core.Runtime that appends metadata dependencies to all files. That should take care of invalidation. Another option is to modify scheduleOutOfDate

I had to prepend some Rules to global Rules set. This might be possible
to replaced by a correct Store.set call.
I also had to prepend some Compile rules.
@krsch
Copy link
Contributor Author

krsch commented Oct 15, 2013

It looks like it is much harder to implement invalidation. Universe doesn't normally include metadata files as they are not compiled, so I had to add a new rule.

@krsch
Copy link
Contributor Author

krsch commented Oct 22, 2013

Should I add anything else in this pull request?

@jaspervdj
Copy link
Owner

Sorry for the late reply, I haven't had time to review this yet. I'll do it tomorrow evening.

@co-dan
Copy link
Contributor

co-dan commented Jan 21, 2014

Any updates on this issue?

@@ -53,7 +54,7 @@ run config verbosity rules = do
provider <- newProvider store (shouldIgnoreFile config) $
providerDirectory config
Logger.message logger "Running rules..."
ruleSet <- runRules rules provider
ruleSet <- runRules (internalRules >> rules) provider
Copy link
Owner

Choose a reason for hiding this comment

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

We probably want to have internalRules last. Earlier rules override later rules, and we want to give the user the option of overwriting them.

@jaspervdj
Copy link
Owner

Looks good to merge! The only thing that might be a bit nicer is if we could now use the internal rules to also do the dependency tracking for the foo.md.metadata files.

@krsch
Copy link
Contributor Author

krsch commented Jan 21, 2014

I have extended global metadata code to handle foo.md.metadata files. If that's what you wanted you can now clean up metadata code.

@jaspervdj
Copy link
Owner

The feature is really coming together, thanks for all the hard work, I really appreciate it! :-)

I might clean it up a bit before releasing it but I'm merging it in now.

jaspervdj added a commit that referenced this pull request Jan 23, 2014
Implemented per-directory metadata support
@jaspervdj jaspervdj merged commit 63107a6 into jaspervdj:master Jan 23, 2014
@jaspervdj
Copy link
Owner

One more question. In namedMetadataBlock, there is the isNamed parameter which allows the first metadata block to be called **, if I read your code correctly. However, I don't think it is used in the test. I'm wondering what the purpose of this feature is?

@krsch
Copy link
Contributor Author

krsch commented Feb 3, 2014

If isNamed is true, a block without filename glob can be matched (e.g. in
some-file.metadata). It matches empty string, so if the first block is
named, one additional block with no metadata is generated.
isNamed was added to support per-file metadata.

2014-02-03 Jasper Van der Jeugt notifications@github.com:

One more question. In namedMetadataBlock, there is the isNamed parameter
which allows the first metadata block to be called **, if I read your
code correctly. However, I don't think it is used in the test. I'm
wondering what the purpose of this feature is?


Reply to this email directly or view it on GitHubhttps://github.com//pull/190#issuecomment-33936126
.

@jaspervdj
Copy link
Owner

Oh, I get it, it's the ** pattern. That's a nice trick. :-)

@jtojnar
Copy link
Contributor

jtojnar commented Jun 21, 2017

Why was this reverted?

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.

4 participants