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

Preliminary, broken support for menus #36

Closed
wants to merge 3 commits into from

Conversation

titaniumbones
Copy link
Contributor

DO NOT MERGE. This commit adds preliminary support for menu metadata
in posts. It sort of works with YAML frontmatter but not with TOML.

I add a few variables, and an extra function that amends the output of org-hugo--gen-front-matter.

I'll try to give better details in a comment on this PR, but late for an appointment and I see you're working on this stuff right now, so want to get this PR out!

DO NOT MERGE. This commit adds preliminary support for menu metadata
in posts. It sort of works with YAML frontmatter but not with TOML.

more info in PR
@titaniumbones
Copy link
Contributor Author

hmm, I think there's a better way to do this. In --gen-front-matter, I definitely miss the info channel. Is there a good reason for separating out --get-front-matter, where data is defined, and --gen-front-matter? To generate the menus I need all the menu-related data...

Anyway, maybe there's a better way. My command of lisp, yaml, and toml is a bit weak (for all 3!), so I am running into multiple stumbling blocks.

@kaushalmodi
Copy link
Owner

In --gen-front-matter, I definitely miss the info channel. Is there a good reason for separating out --get-front-matter, where data is defined, and --gen-front-matter?

It's mainly for code readability and portability.

(defun org-hugo--get-front-matter (info)

The idea is that org-hugo--get-front-matter gets all the stuff it can from info in yaml/toml agnostic matter and add that stuff to data.

(defun org-hugo--gen-front-matter (data format)

org-hugo--gen-front-matter then takes that data and outputs a string based on the set format (TOML/YAML).

To generate the menus I need all the menu-related data...

That should still be possible as long as you add all you need to the data var. Do you see a limitation in trying to extract menu related meta data and add that to data?

Anyway, maybe there's a better way.

I don't know.. I'll have a look at this next. :)

My command of lisp, yaml, and toml is a bit weak (for all 3!), so I am running into multiple stumbling blocks.

No worries. I'll look at this (I am not yaml/toml expert, but my understanding is that the formatting done in org-hugo--gen-front-matter is probably all that should be needed).

ox-hugo.el Outdated
@@ -126,7 +126,11 @@ directory where all Hugo posts should go by default."
(:hugo-section "HUGO_SECTION" nil org-hugo-default-section-directory)
(:hugo-base-dir "HUGO_BASE_DIR" nil nil)
(:hugo-static-images "HUGO_STATIC_IMAGES" nil "images")
(:hugo-code-fence "HUGO_CODE_FENCE" nil "t")))
(:hugo-code-fence "HUGO_CODE_FENCE" nil "t")
(:hugo-static-images "HUGO_STATIC_IMAGES" "images" nil)
Copy link
Owner

Choose a reason for hiding this comment

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

Looks like this got re-adding by mistake when you resolved the merge conflict. I had fixed the order of the default value. The correct order is:

;; KEY KEYWORD OPTION DEFAULT BEHAVIOR

So you have the "images" and nil swapped.

ox-hugo.el Outdated
(:hugo-code-fence "HUGO_CODE_FENCE" nil "t")
(:hugo-static-images "HUGO_STATIC_IMAGES" "images" nil)
(:hugo-menu-name "HUGO_MENU_NAME" "main" nil)
(:hugo-menu-weight "HUGO_MENU_WEIGHT" 20 nil)
Copy link
Owner

Choose a reason for hiding this comment

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

Same swapping of 20 and nil here. I'd be surprised if the default value of menu weight showed up to be 20 for you.

ox-hugo.el Outdated
(:hugo-code-fence "HUGO_CODE_FENCE" nil "t")))
(:hugo-code-fence "HUGO_CODE_FENCE" nil "t")
(:hugo-static-images "HUGO_STATIC_IMAGES" "images" nil)
(:hugo-menu-name "HUGO_MENU_NAME" "main" nil)
Copy link
Owner

Choose a reason for hiding this comment

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

Same swapping

@kaushalmodi
Copy link
Owner

It would help to have an example added for this new feature too, so that I have an exact idea of what you'd like to put in the Org file and what the output .md should look like. Something to note for all future commits and PRs.

But for this one I think I can figure out the example.

ox-hugo.el Outdated

(if (string= format "yaml")
(let ((indent " "))
(format "\nmenu:\n %s:\n %s: %s\n %s: %s\n %s: %s\n"
Copy link
Owner

Choose a reason for hiding this comment

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

Looks like you plan to have just one "menu name" per post? I will try to get this working for just that for now. Supporting multiple menu names (and their hierarchies) dynamically seems a bit complicated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. I agree, since the yaml & toml will end up looking very different.

@kaushalmodi
Copy link
Owner

kaushalmodi commented Jul 17, 2017

I started working on this, but it's still not finished; you can see the last 2 commits in this branch: https:/kaushalmodi/ox-hugo/commits/support-menus

The only missing piece is the translation from the menu meta data collected in value to YAML/TOML string on line 524.

Adds ability to construct toml and yaml front matter, as well as
primitive testing in example-site, and a couple of extra lines to config.toml.
@titaniumbones
Copy link
Contributor Author

I built on your work (muddied up the commit history again -- sorry I am a bit slow learning to fix up commits) so that menus now seem to be constructed properly in both YAML and TOML.

We probably need a minimal menu.html partial to display the tests, though.

:EXPORT_DATE: 2017/07/17
:HUGO_MENU_NAME: main
:HUGO_MENU_WEIGHT: 100
:HUGO_MENU_PARENT: posts
Copy link
Owner

Choose a reason for hiding this comment

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

About this, I was thinking of changing the way of representing this in Org. Then a user can have a menu variable with any name they like (example: you had a custom var called identifier).

How about:

#+HUGO_MENU: :name main :weight 100 :parent posts :foo bar

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think there's a problem with this schema. SUppose I have a large org file with many posts. Most of the tie I just let the menu be constructed with default values for all menu attributes. However, for a few posts I want to change the menu weight (the posts are important and I want them to come at the top of the menu). In order to make that one change, I have to correctly transcribe all the menu values and change only the one I care about.

The old schema (as written in my commit) is a bit clumsy but maybe it's ultimately more flexible for the end-user.

Copy link
Owner

Choose a reason for hiding this comment

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

I think there's a problem with this schema. SUppose I have a large org file with many posts. Most of the tie I just let the menu be constructed with default values for all menu attributes. However, for a few posts I want to change the menu weight (the posts are important and I want them to come at the top of the menu). In order to make that one change, I have to correctly transcribe all the menu values and change only the one I care about.

Would it help to have HUGO_MENU and something like HUGO_MENU_OVERRIDE? I would like to remove hard-coding of all user menu variables.

Another option is to pre-fill a new post subtree with HUGO_MENU property value from any parent in the helper function (to be implemented).

The old schema (as written in my commit) is a bit clumsy but maybe it's ultimately more flexible for the end-user.

The primary reason for proposing the new schema is that I want to avoid hard-coding user variables like identifier and parent in ox-hugo. (See below)

(menu (when menu-name
                 (cons menu-name `((identifier . ,menu-identifier) (parent . ,menu-parent) (weight . ,menu-weight)))))

@kaushalmodi
Copy link
Owner

kaushalmodi commented Jul 18, 2017

I built on your work (muddied up the commit history again -- sorry I am a bit slow learning to fix up commits) so that menus now seem to be constructed properly in both YAML and TOML.

Thanks from picking it up from where I left.

Looks fine to me.

image

We probably need a minimal menu.html partial to display the tests, though.

Yes, I mention that here. You can commit the partial that you are using to the example-site if you like.


Update

Actually I realized that there was a minor commit messup.. your branch wasn't based off the latest master any more. I have fixed that by cherry-picking your commit now; pushed to my support-menus branch.

image

@kaushalmodi
Copy link
Owner

@titaniumbones Please update to my support-menus branch. I have done some bug fixes and cleanup in the code and the examples.

Now we just need to converge on the menu specification in Org as I'd like to remove hard-coding of user menu variables. Let me know if one of these would work for you:

  1. Have HUGO_MENU and HUGO_MENU_OVERRIDES. So you can set the default menu params in HUGO_MENU and override just weight or anything you like in HUGO_MENU_OVERRIDES.
  2. Have just HUGO_MENU, but then use a helper function to pre-fill the HUGO_MENU property from a parent (if present) when creating a new sub-tree for a new post.

Any other ideas you have to prevent the hard-coding of parent, identifier, etc in the current schema?

@titaniumbones
Copy link
Contributor Author

@titaniumbones Please update to my support-menus branch. I have done some bug fixes and cleanup in the code and the examples.

OK thx will do.

Now we just need to converge on the menu specification in Org as I'd like to remove hard-coding of user menu variables. Let me know if one of these would work for you:
Have HUGO_MENU and HUGO_MENU_OVERRIDES. So you can set the default menu params in HUGO_MENU and override just weight or anything you like in HUGO_MENU_OVERRIDES.
Have just HUGO_MENU, but then use a helper function to pre-fill the HUGO_MENU property from a parent (if present) when creating a new sub-tree for a new post.
Any other ideas you have to prevent the hard-coding of parent, identifier, etc in the current schema?

A couple of thoughts:

  • my issue with "identifier" was due to a failure to wrap TOML values in quotes. So that one should be less of an issue than I'd thought.
  • I'm not sure exactly how the new schema prevents hard-coding. The old schema also allows inheritance of HUGO_* properties, so these can be declared at the top of the file as well as in a property drawer. But do you mean osmething else?

I definitely agree in principle that a main goal should be minimizing user labour and maximizing the set of org features that we support!

@kaushalmodi
Copy link
Owner

kaushalmodi commented Jul 18, 2017

my issue with "identifier" was due to a failure to wrap TOML values in quotes. So that one should be less of an issue than I'd thought.

I have generalized the whole thing as you see. Also now in the new branch menu-props-like-header-args, I added support for all Hugo menu properties out there! :)

I'm not sure exactly how the new schema prevents hard-coding.

Earlier I thought that 'identifier', 'parent' were menu properties that you used in your personal site; but as I learn later, they are part of Hugo menu site. In any case, the new schema is more flexible; you do not need to define a new property for each menu property.

Now it is dynamic:

(dolist (prop '(name url menu identifier pre post weight parent children))

The old schema also allows inheritance of HUGO_* properties, so these can be declared at the top of the file as well as in a property drawer. But do you mean osmething else?

Yes, sort of.. see this example. If I do not have the additional HUGO_MENU_OVERRIDE, I cannot do partial overrides as I do in the above example.

I definitely agree in principle that a main goal should be minimizing user labour and maximizing the set of org features that we support!

I hope you like this feature then! I have a bunch of examples using the new schema (the above, and also this (search for "menu" in here)).

@titaniumbones
Copy link
Contributor Author

Hmm. I think that new implementation seems great. Haven't tested it yet but do you want to go ahead and merge your new branch? Obviously we're getting close.

I'm seeing some odd behaviour (inconsistent relinking of images -- works on the example site, but not ion my real site) and a little distracted trying to track that down. Sorry about that!.

@kaushalmodi
Copy link
Owner

Hmm. I think that new implementation seems great. Haven't tested it yet but do you want to go ahead and merge your new branch? Obviously we're getting close.

Cool. I have merged it. In addition, I also made this one fix (
8068f34 ); we were confusing the menu "name" property with the menu entry name. Now the menu entry name like "main" is set using ":menu main" instead of ":name main".

I'm seeing some odd behaviour (inconsistent relinking of images -- works on the example site, but not ion my real site) and a little distracted trying to track that down. Sorry about that!.

What is re-linking of images? Is that the copying of images done to the static dir? I keep all images in static/ dir to begin with and link to those in Org. So I haven't heavily tested the attachment moving fn if that's what you referred to.

@kaushalmodi
Copy link
Owner

Closing this PR. I have not merged it directly, but I have cherry-picked the commits from here.

Thanks!

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.

2 participants