jskherman / imprecv

A no-frills curriculum vitae (CV) template using Typst and YAML to version control CV data.
Apache License 2.0
319 stars 38 forks source link

Modular file structure #2

Closed mangkoran closed 1 year ago

mangkoran commented 1 year ago

Hello I am back :D I couldn't stop myself from tinkering with Typst.

This PR is a bigger one. This PR would like to achieve "modularity" (I don't really sure this is the right terms). The main goal is to allow user to keep the template "updated" or at least "in line" with the upstream by adding this repo as a git submodule and use/import the "base" or "style" file (cv.typ). This way, user would be able to add their own style rules or even override existing rules in their "content" file (content.typ, user can use their own "content" file).

user-cv.typ/
├── cv.typ/ // added as git submodule so update can be done easily
|   └── cv.typ
└── content.typ // #import "cv.typ/cv.typ": *

To achieve this, several works have been done:

Additionally, there is one WIP stuff which is how we can pass variables for showAddress and showNumber and read the value in cv.typ. Currently I am reading Typst's dictionary reference and I think dictionary reassignment could be used here.

As always, please let me know if you have any comments or want me to add something. I am also fine if this PR "contradicts" your ideals for the structure of this template, and I might continue this PR as a fork 😉

mangkoran commented 1 year ago

Commit 7ce2f81 (#2) now fix variable showAddress and showNumber passing 🎉🎉🥳🥳

Please let me know if anything should be changed 😉

jskherman commented 1 year ago

Sure! It's always nice to get suggestions on how to improve the code.

I see how it would be useful to add an abstraction over each part for the git submodule and ensure consistency for future updates, albeit making the code for constructing the document less obvious. A PR of this size though might take me some time to review. Let me get back to you once I try the changes.

P.S. Yeah, I believe the right term is modularity.

mangkoran commented 1 year ago

Thank you for sparing your time! Please let me know before you're going to merge it, as I might have something to add or tidy up (I am digging into the Typst rabbit hole :D)

jskherman commented 1 year ago

Ok 👍, take care to not to fall too deep in the rabbit hole :3 (just looking at the plotting libraries will make you feel you're staring into the abyss).

Edit: regarding the git submodule, perhaps making a package with Typst might be preferable?

mangkoran commented 1 year ago

Ok 👍, take care to not to fall too deep in the rabbit hole :3 (just looking at the plotting libraries will make you feel you're staring into the abyss).

I will try my best to not to. My attention span is so low I easily distracted with interesting things 😅

Edit: regarding the git submodule, perhaps making a package with Typst might be preferable?

Wow this is great. I will try to read it first. But for now, I am going to settle with the submodule approach. If the Typst package is better and doable, we could revisit it later on :D

mangkoran commented 1 year ago

Commit d053d3f (#2) now move the CV data loading out to user-specified "content" file (content.typ for example purpose).

mangkoran commented 1 year ago

Additionally, I will create a commit to update the readme to reflect this change.

jskherman commented 1 year ago

The code for the functions is all good but I believe something has changed the line spacings to create loose unordered lists, but I can't seem to pinpoint what it is the code. This results in the projects section being truncated and the have more whitespace between the links section and education section.

image

mangkoran commented 1 year ago

This results in the projects section being truncated and the have more whitespace between the links section and education section.

Sorry but I couldn't reproduce the wide spacings as you provided. To make sure, please checkout my latest commit. I just reworded the commits so it shouldn't affect anything.

Additionally, we could add rules to prevent section title truncation. But that could be done in different PR.

image

jskherman commented 1 year ago

Huh, weird. I see, maybe it's on my end. At one point, the output seems to have been the same and then different the next.

Anyways, I believe everything is in order. I would like to suggest though some minor style changes in using curly brackets {} instead of [] for the surrounding functions for the different "modules", if there are no content at the same level as the inner functions/commands.

Edit: Maybe this could be left for another PR/change instead, along with other chores.

jskherman commented 1 year ago

If you have no other changes to include, then I think we can merge this PR?

mangkoran commented 1 year ago

Huh, weird. I see, maybe it's on my end. At one point, the output seems to have been the same and then different the next.

Try to use typst compile instead of typst watch. I also experience "wrong content" compilation. It could be because I am using nvim (see https://github.com/typst/typst/issues/1693).

I would like to suggest though some minor style changes in using curly brackets {} instead of [] for the surrounding functions for the different "modules", if there are no content at the same level as the inner functions/commands.

I think I could include it in this PR. Could you please left comments in the code so that I could locate the exact styling that you meant? If it's on many place, please do "Start a review".

image

mangkoran commented 1 year ago

Let's wait until tomorrow before merging. I still haven't updated the documentation 🤣

mangkoran commented 1 year ago

Hi @jskherman. Sorry but today was a busy day for me. I will try to update the docs by tomorrow.

jskherman commented 1 year ago

Hi @jskherman. Sorry but today was a busy day for me. I will try to update the docs by tomorrow.

Of course, take your time and whenever you're free.

mangkoran commented 1 year ago

Hi @jskherman. I have pushed commits for docs update and files rename. Please take a look on it and let me know if you have any suggestions on it.

You may merge it if you think it is good as currently I have no other additions (might come back again later tho lol).

mangkoran commented 1 year ago

Thank you very much for merging this (and previous) PRs. I am trying to contribute to communities as much as I can do as a payback for such awesome tools they've created. I hope you won't get bored with my future PRs as well 😁

jskherman commented 1 year ago

Thank you too, I appreciate the help as well. It's always nice to have some opportunity to interact in the spirit of open-source.