> 未分类 > [PR#901] feat: enhance tool change widget with variables support

[PR#901] feat: enhance tool change widget with variables support

未分类 bolang 6个月前 (10-14) 48次浏览

Issue #901 | 状态: 已关闭 | 作者: cheton | 创建时间: 2025-01-20

标签: Review effort [1-5]: 2

PR Type

Enhancement

_

Description

– Added insertAtCaret utility for text insertion at caret position.

– Introduced variables list for tool change and probe variables.

– Enhanced Tool widget with dropdown for inserting variables.

– Removed unused toolprobeoverrides property from multiple controllers.

_

Changes walkthrough 📝

Relevant files
Enhancement

insertAtCaret.js

Add utility for text insertion at caret                                   

src/app/widgets/Tool/insertAtCaret.js

  • Added a utility function insertAtCaret.
  • Allows text insertion at caret position in textareas.

    		•  +14/-0   
    variables.js

    Add predefined tool change and probe variables                     

    src/app/widgets/Tool/variables.js

  • Added a list of tool change and probe variables.
  • Organized variables into categories with headers.

    		•  +30/-0   
    GrblController.js

    Remove unused toolprobeoverrides in GrblController         

    src/server/controllers/Grbl/GrblController.js

    – Removed unused toolprobeoverrides property.

    		 +0/-1     
    MarlinController.js

    Remove unused toolprobeoverrides in MarlinController     

    src/server/controllers/Marlin/MarlinController.js

    – Removed unused toolprobeoverrides property.

    		 +0/-1     
    SmoothieController.js

    Remove unused toolprobeoverrides in SmoothieController 

    src/server/controllers/Smoothie/SmoothieController.js

    – Removed unused toolprobeoverrides property.

    		 +0/-1     
    TinyGController.js

    Remove unused toolprobeoverrides in TinyGController       

    src/server/controllers/TinyG/TinyGController.js

    – Removed unused toolprobeoverrides property.

    		 +0/-1     
    Tool.jsx

    Enhance Tool widget with variable dropdown and UI improvements

    src/app/widgets/Tool/Tool.jsx

  • Integrated insertAtCaret utility for text insertion.
  • Added dropdown for inserting predefined variables.
  • Enhanced UI for custom tool probe commands.
  • Added support for editable text fields with variable insertion.

    		•  +64/-8   

    _

    >

    Need help?
  • Type /help how to ... in the comments thread for any question about Qodo Merge usage.
  • Check out the documentation for more information.

    • 评论 (3)

    #1 – codesandbox[bot] 于 2025-01-20

    #### Review or Edit in CodeSandbox

    Open the branch in source=ghapp”>VS Code • source=ghapp”>Preview

    #2 – codiumai-pr-agent-free[bot] 于 2025-01-20

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Potential Memory Leak

    The component stores DOM node references in the fields object but doesn’t clean them up in componentWillUnmount. This could potentially cause memory leaks.

     

    fields = {
  toolProbeOverrides: null,
};

    Input Validation

    The insertAtCaret function doesn’t validate if the textarea parameter is a valid DOM element before accessing its properties, which could cause runtime errors.

     

    const insertAtCaret = (textarea, text = '') => {
  const scrollPos = textarea.scrollTop;
  const caretPos = textarea.selectionStart;
  const front = (textarea.value).substring(0, caretPos);
  const back = (textarea.value).substring(textarea.selectionEnd, textarea.value.length);
  textarea.value = front + text + back;
  textarea.selectionStart = caretPos + text.length;
  textarea.selectionEnd = caretPos + text.length;
  textarea.focus();
  textarea.scrollTop = scrollPos;
};

    #3 – codiumai-pr-agent-free[bot] 于 2025-01-20

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    Category Suggestion                                                                                                                                    Score
    Possible issue

    Add input parameter validation

    _

    Add input validation to ensure textarea parameter is a valid DOM element before
    performing operations. This prevents runtime errors if an invalid element is passed.

    [src/app/widgets/Tool/insertAtCaret.js [2-4]](https://github.com/cncjs/cncjs/pull/901/files#diff-cf44530dca7dbc629f9ec03233e9551fb33b93d6e1fc737e1a39a839bb78317dR2-R4)

     const insertAtCaret = (textarea, text = '') => {
+  if (!textarea || !(textarea instanceof HTMLTextAreaElement)) {
+    return;
+  }
   const scrollPos = textarea.scrollTop;
   const caretPos = textarea.selectionStart;

    – [ ] Apply this suggestion

    Suggestion importance[1-10]: 8

    Why: Adding input validation is crucial for preventing runtime errors and improving code robustness. The suggestion would prevent potential crashes when invalid parameters are passed to this utility function.

    		 8
    General

    Cleanup component references properly

    _

    The toolProbeOverrides ref should be properly cleaned up when the component unmounts
    to prevent memory leaks and potential null reference errors.

    [src/app/widgets/Tool/Tool.jsx [123-125]](https://github.com/cncjs/cncjs/pull/901/files#diff-a401a7973526e77e8931e59448c9404d71669a3b2e31a615ef726bce6f023aecR123-R125)

    +componentWillUnmount() {
+  this.fields.toolProbeOverrides = null;
+}
+
 fields = {
   toolProbeOverrides: null,
 };

    – [ ] Apply this suggestion

    Suggestion importance[1-10]: 3

    Why: While cleaning up refs is good practice, in this case the component will be destroyed anyway and React will handle the cleanup. The impact is minimal since there’s no risk of memory leaks in this specific implementation.

    		 3

    原始Issue: https://github.com/cncjs/cncjs/pull/901

    喜欢 (0)