HDD-1443 export ready document with indents#2
Merged
jack-happydoc merged 3 commits intomasterfrom Jan 10, 2025
Merged
Conversation
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 72958a6 in 52 seconds
More details
- Looked at
46lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. lib/rtf.js:40
- Draft comment:
The variableelementshould be declared usingvar,let, orconstto 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) { | |||
There was a problem hiding this comment.
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); |
davemilller
approved these changes
Jan 10, 2025
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
clearly never worked
Important
Fix
writeTextfunction inlib/rtf.jsto handle group elements correctly and update version inpackage.json.writeText()inlib/rtf.jsto useaddElement()instead ofpush()for group elements._groupIndex()inlib/rtf.jsto return the correct index of a group.package.jsonfrom0.0.1to0.0.2.This description was created by
for 72958a6. It will automatically update as commits are pushed.