refactor #19
refactor #19
Conversation
|
Alright I think it's now ready for review. No issue so far on my end. I think we need to update the JSON schema too as we made some fields optional, but I will revisit it later. Please let me know if you have any suggestions. Friendly mention @jskherman @kyawzazaw @phamson02 |
|
I also have an idea to split input files/YAML for sensitive data such as phone number, email, location etc. So we could track the other input file while keeping the sensitive data locally. What do you think about it? |
cv.typ
Outdated
| }) | ||
|
|
||
| for hi in p.highlights { | ||
| [- #hi] |
There was a problem hiding this comment.
Hm, this part should be reverted back to using eval(). This breaks the use of markup like bold or italics when present in the YAML file.
| [- #hi] | |
| for hi in p.highlights{ | |
| [- #eval(hi, mode: "markup")] | |
| } |
cv.typ
Outdated
| for h in org.highlights { | ||
| [- #h] | ||
| } |
There was a problem hiding this comment.
Similar problem here as well as above...
| for h in org.highlights { | |
| [- #h] | |
| } | |
| for hi in org.highlights { | |
| [- #eval(hi, mode: "markup")] | |
| } |
cv.typ
Outdated
| for h in project.highlights { | ||
| [- #h] | ||
| } |
There was a problem hiding this comment.
| for h in project.highlights { | |
| [- #h] | |
| } | |
| for hi in project.highlights { | |
| [- #eval(hi, mode: "markup")] | |
| } |
cv.typ
Outdated
| for h in award.highlights { | ||
| [- #h] | ||
| } |
There was a problem hiding this comment.
| for h in award.highlights { | |
| [- #h] | |
| } | |
| for hi in award.highlights { | |
| [- #eval(hi, mode: "markup")] | |
| } |
cv.typ
Outdated
| for skill in skills { | ||
| dot_list(skill.category, skill.skills) | ||
| } |
There was a problem hiding this comment.
To reduce confusion, might I suggest to use the variable name group, skill-group, or skill_group?
| for skill in skills { | |
| dot_list(skill.category, skill.skills) | |
| } | |
| for skill_group in skills { | |
| dot_list(skill_group.category, skill_group.skills) | |
| } |
cv.typ
Outdated
| let foo = () | ||
| foo.push(ref.reference) | ||
| dot_list(ref.name, foo, url: ref.url) |
There was a problem hiding this comment.
May I ask if foo here is needed? Could it instead be an array created directly?
| let foo = () | |
| foo.push(ref.reference) | |
| dot_list(ref.name, foo, url: ref.url) | |
| dot_list(ref.name, (ref.reference,), url: ref.url) |
Hm, technically you could consider all of the data in a résumé or CV sensitive data. Also, it might get a bit too unwieldy to have the data scattered in different YAML files. To illustrate, my current use case involves using different YAML files slightly altered from a main YAML file (used in the CV) to create one/two-page résumés that are tailored to a specific job description or role. For me, having one YAML file per document makes it easier to diff these customized "versions". |
|
Hi @jskherman I apologize for the late reply as I am busy with with IRL. Originally this PR should be opened as a draft PR as it still need some rework. And thank you for leaving suggestions. However, I'm planning to do some additional refactor to comply with Typst's template format. I think I will still use this PR for that. |
|
Suggestions committed. Could you please recheck whether it's what you requested? Meanwhile I will continue to update the project structure to match template format. |
This allows user to remove fields they want to hide entirely
In most packages (e.g. tablex) functions are named after snake_case and variables are named after kebab-case
By removing checks, we enforce the usage of some variables that are deemed to be mandatory. This commit also remove section data checks. So if user don't include the respective section data, it implies the user don't want to include the data and they need to exclude the respective section function call in the template in order to compile.
After second check, it seems all of function and variable use kebab-case
In my opinion, targeting master branch for a schema would be an issue if we introduce a breaking changes to the schema. By using local schema, it ensures that the current schema is in line with current cv.typ version.
|
Hi @mangkoran, sorry for only getting back to you at this time since I had a big exam to prepare for in the past few months... I'm sorry to spring this on you only now after you have done a lot of work on this PR. But I was thinking of closing this PR and doing a double check on the code base as a whole for consistency (including reacquainting myself with the code after being away for a while) and, while at it, convert the setup to use the new system for templates in the Typst Universe. To be honest, it also feels rather daunting to review such a huge PR since I'm only coding as a hobby (skill issue, I know). I'm quite thankful though for the amount of work you put into this. |
WIP. I decided to do some refactor of the codebase so it looks more neat (IMO). Apparently it will solve #1 (comment) #9, #10 but not yet for now.
Current state/commit is on parity with master (no failed compilation).
Please let me know if you have any suggestions.