-
-
Notifications
You must be signed in to change notification settings - Fork 23
New features #10
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
base: master
Are you sure you want to change the base?
New features #10
Conversation
… JSON, and Upload of BibTex accepts .bib and .txt.
…ore author names.
…in preview for the paper size (but won't print).
Citation format works correctly.
… for publications to include new fields (authors, citationKey, bibtexEntry) and improved handling of formatted citations. Modified BibTeX processing to support replace/merge modes for publications. Adjusted citation formatting in the UI to ensure proper display and alignment. Cleaned up CSS for citation layout.
…lication year if not available, and implemented sorting of publications by release date (newest first) in the CV data. Improved debug logging for better visibility of publication updates.
/gemini review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request introduces new features like BibTeX support and theme selection. The review focuses on correctness, maintainability, and robustness, identifying issues such as incorrect citation numbering, inefficient BibTeX handling, and external dependencies. Addressing these will improve reliability and maintainability.
const publications = cite.data.map((entry, index) => { | ||
console.log(`[Debug] Entry ${index} type:`, entry.type) | ||
|
||
// Format individual citation | ||
const singleCite = new Cite(entry) | ||
const formattedHTML = singleCite.format('bibliography', { | ||
format: 'html', | ||
template: 'ieee', | ||
lang: 'en-US' | ||
}) | ||
|
||
// Clean up the HTML structure | ||
const processedHTML = formattedHTML | ||
.replace(/\r?\n|\r/g, '') // Remove all newlines | ||
.replace(/>\s+</g, '><') // Remove whitespace between tags | ||
.replace(/\s+/g, ' ') // Replace multiple spaces with single space | ||
.replace(/, doi: .*?(?=<\/div>)/g, '') // Remove DOI from citation text | ||
.trim() // Remove leading/trailing whitespace |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The current implementation generates formattedHTML
incorrectly, resulting in all citations being numbered as [1]
. Instead, use the citationDivs
variable, which already contains the correctly formatted and numbered entries from the initial bibliography formatting.
const processedHTML = citationDivs[index] || '';
// Group publications by type, defaulting to 'other' if type not specified | ||
const journalPubs = cv.publications?.filter(p => p.type === 'journal') || [] | ||
const conferencePubs = cv.publications?.filter(p => p.type === 'conference') || [] | ||
const otherPubs = cv.publications?.filter(p => !p.type || p.type === 'other') || [] | ||
|
||
// Only show section if there are publications | ||
if (hasItems(journalPubs) || hasItems(conferencePubs) || hasItems(otherPubs)) { | ||
%> | ||
<section class="publications-section"> | ||
<div class="section-title"> | ||
<h2>Publications</h2> | ||
<div class="line"></div> | ||
</div> | ||
|
||
<% if (hasItems(journalPubs)) { %> | ||
<div class="subsection"> | ||
<h3>Journal Papers</h3> | ||
<% for (let i = 0; i < journalPubs.length; i++) { %> | ||
<div class="publication section-item"> | ||
<div class="citation"><%- formatCitation(journalPubs[i], i) %></div> | ||
<%- linkInDiv(journalPubs[i].url) %> | ||
</div> | ||
<% } %> | ||
</div> | ||
<% } %> | ||
|
||
<% if (hasItems(conferencePubs)) { %> | ||
<div class="subsection"> | ||
<h3>Conference Papers</h3> | ||
<% for (let i = 0; i < conferencePubs.length; i++) { %> | ||
<div class="publication section-item"> | ||
<div class="citation"><%- formatCitation(conferencePubs[i], i) %></div> | ||
<%- linkInDiv(conferencePubs[i].url) %> | ||
</div> | ||
<% } %> | ||
</div> | ||
<% } %> | ||
|
||
<% if (hasItems(otherPubs)) { %> | ||
<div class="subsection"> | ||
<h3>Other Publications</h3> | ||
<% for (let i = 0; i < otherPubs.length; i++) { %> | ||
<div class="publication section-item"> | ||
<div class="citation"><%- formatCitation(otherPubs[i], i) %></div> | ||
<%- linkInDiv(otherPubs[i].url) %> | ||
</div> | ||
<% } %> | ||
</div> | ||
<% } %> | ||
</section> | ||
<% } %> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const publications = await processBibTeX(bibtexStr, currentData) | ||
|
||
// Get current mode from radio buttons | ||
const shouldReplace = $('input[name="bibtex-mode"]:checked').val() === 'replace' | ||
const newData = updatePublications(currentData, publications, shouldReplace) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The BibTeX processing logic is inefficient. processBibTeX
is called, and its result is stored in publications
. Then, updatePublications
calls processBibTeX
again. Simplify to a single call to processBibTeX
with the correct shouldReplace
parameter. The updatePublications
function is then redundant and can be removed from src/lib/bibtex.js
.
const publications = await processBibTeX(bibtexStr, currentData) | |
// Get current mode from radio buttons | |
const shouldReplace = $('input[name="bibtex-mode"]:checked').val() === 'replace' | |
const newData = updatePublications(currentData, publications, shouldReplace) | |
const shouldReplace = $('input[name="bibtex-mode"]:checked').val() === 'replace'; | |
const newData = await processBibTeX(bibtexStr, currentData, shouldReplace); |
import { plugins } from '@citation-js/core' | ||
|
||
// Fetch and register IEEE CSL template | ||
fetch('https://raw.githubusercontent.com/citation-style-language/styles/master/ieee.csl') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
function _parsePublication(pub) { | ||
if (typeof pub === 'string') { | ||
try { | ||
return JSON.parse(pub) | ||
} catch (e) { | ||
console.error('Failed to parse publication:', e) | ||
return pub | ||
} | ||
} | ||
return pub | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const keyFormatMap = { | ||
// Basic fields | ||
'basics.properties.summary': 'textarea', | ||
|
||
// add format to schema | ||
const keyFormatMap = { | ||
'basics.properties.summary': 'textarea', | ||
'work.items.properties.description': 'textarea', | ||
'work.items.properties.summary': 'textarea', | ||
'work.items.properties.highlights.items': 'textarea', | ||
'projects.items.properties.description': 'textarea', | ||
'projects.items.properties.highlights.items': 'textarea', | ||
'sideProjects.items.properties.description': 'textarea', | ||
'skills.items.properties.summary': 'textarea', | ||
'languages.items.properties.summary': 'textarea', | ||
'references.items.properties.reference': 'textarea', | ||
'awards.items.properties.summary': 'textarea', | ||
'publications.items.properties.summary': 'textarea', | ||
'volunteer.items.properties.summary': 'textarea', | ||
'volunteer.items.properties.highlights.items': 'textarea', | ||
} | ||
for (const [key, format] of Object.entries(keyFormatMap)) { | ||
objectPath.get(jsoncvSchema.properties, key).format = format | ||
} | ||
// Work and projects | ||
'work.items.properties': { | ||
'description': 'textarea', | ||
'summary': 'textarea', | ||
'highlights.items': 'textarea' | ||
}, | ||
'projects.items.properties': { | ||
'description': 'textarea', | ||
'highlights.items': 'textarea' | ||
}, | ||
'sideProjects.items.properties.description': 'textarea', | ||
|
||
// Skills and qualifications | ||
'skills.items.properties.summary': 'textarea', | ||
'languages.items.properties.summary': 'textarea', | ||
'references.items.properties.reference': 'textarea', | ||
|
||
// Awards and recognition | ||
'awards.items.properties.summary': 'textarea', | ||
'publications.items.properties.summary': 'textarea', | ||
'volunteer.items.properties': { | ||
'summary': 'textarea', | ||
'highlights.items': 'textarea' | ||
}, | ||
|
||
// change schema title | ||
jsoncvSchema.title = 'CV Schema' | ||
// Academic fields | ||
'researchAreas.items.properties': { | ||
'description': 'textarea', | ||
'contributions.items': 'textarea' | ||
}, | ||
'academicAppointments.items.properties.summary': 'textarea', | ||
'teaching.items.properties.description': 'textarea', | ||
'grants.items.properties.summary': 'textarea', | ||
'mentoring.items.properties.summary': 'textarea', | ||
'professionalServices.items.properties.details': 'textarea', | ||
'presentations.items.properties.description': 'textarea', | ||
'software.items.properties.description': 'textarea' | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Features:
meta.pageSize
forletter
andA4
, but no UI changes have been made.Fixes
To do: