Skip to content

Conversation

@jack-happydoc
Copy link

@jack-happydoc jack-happydoc commented Jan 10, 2025

clearly never worked


Important

Fix writeText function in lib/rtf.js to handle group elements correctly and update version in package.json.

  • Behavior:
    • Fix writeText() in lib/rtf.js to use addElement() instead of push() for group elements.
    • Correct _groupIndex() in lib/rtf.js to return the correct index of a group.
  • Versioning:
    • Update version in package.json from 0.0.1 to 0.0.2.

This description was created by Ellipsis for 72958a6. It will automatically update as commits are pushed.

Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

❌ Changes requested. Reviewed everything up to 72958a6 in 52 seconds

More details
  • Looked at 46 lines of code in 2 files
  • Skipped 0 files when reviewing.
  • Skipped posting 2 drafted comments based on config settings.
1. lib/rtf.js:40
  • Draft comment:
    The variable element should be declared using var, let, or const to avoid it becoming a global variable.
    const element = new TextElement(text, format);
  • Reason this comment was not posted:
    Comment was on unchanged code.
2. lib/rtf.js:43
  • Draft comment:
    The 'else' block is misaligned. It should be aligned with the 'if' block for better readability.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The indentation of the 'else' block is inconsistent with the rest of the code. It should be aligned properly for better readability.

Workflow ID: wflow_HHR6t2vmxIsMDFB3


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@@ -92,12 +92,15 @@ RTF.prototype.addPar = function (groupName) {
//gets the index of a group
//TODO: make this more private by removing it from prototype and passing elements
RTF.prototype._groupIndex = function (name) {
Copy link

Choose a reason for hiding this comment

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

Consider using findIndex instead of forEach to find the index of a group. It is more efficient and concise.

Suggested change
RTF.prototype._groupIndex = function (name) {
return this.elements.findIndex(el => el instanceof GroupElement && el.name === name);

@jack-happydoc jack-happydoc merged commit 6cd3a3a into master Jan 10, 2025
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.

3 participants