vimco icon indicating copy to clipboard operation
vimco copied to clipboard

Refactor vimco-parse-line and vimco-line-to-faces into smaller, more readable functions

Open Copilot opened this issue 4 months ago • 2 comments

Summary

This PR refactors the vimco-parse-line and vimco-line-to-faces functions by splitting them into smaller, more focused helper functions to improve code readability and maintainability.

Changes

Part 1: vimco-parse-line Refactoring

The original vimco-parse-line function was doing multiple things at once:

  1. Extracting text from a buffer region
  2. Splitting the text by spaces
  3. Parsing individual words into key-value pairs

This has been refactored into three focused functions:

1. vimco-get-region-text

Extracts text from a buffer region:

(defun vimco-get-region-text (region)
  "Extract text from buffer REGION.

REGION should be a cons cell (beg . end) where beg and end are
character positions."
  (let ((beg (car region))
        (end (cdr region)))
    (buffer-substring-no-properties beg end)))

2. vimco-parse-word

Parses a single word into a key-value cons cell:

(defun vimco-parse-word (word)
  "Parse a single WORD into a key-value cons cell.

If WORD contains '=', split it into (key . value).
Otherwise, return (word . nil)."
  (if (string-match "=" word)
      (let ((data (split-string word "=" t)))
        (cons (car data) (cadr data)))
    (cons word nil)))

3. Updated vimco-parse-line

Now orchestrates the parsing using the helper functions:

(defun vimco-parse-line (region)
  "Parse substring from characters in cons REGION into an alist.

REGION which should look like (beg . end), where both are
character numbers. Empty properties have cdr set to nil."
  (let ((text (vimco-get-region-text region)))
    (mapcar #'vimco-parse-word
            (split-string text " " t))))

Part 2: vimco-line-to-faces Refactoring

The original vimco-line-to-faces function was performing multiple complex operations:

  1. Normalizing property values
  2. Processing attributes strings
  3. Formatting color properties
  4. Building face attributes plists
  5. Creating face definitions

This has been refactored into five focused helper functions:

1. vimco-normalize-property-value

Normalizes property values by replacing "NONE" strings with nil:

(defun vimco-normalize-property-value (value)
  "Normalize property VALUE by replacing \"NONE\" with nil.

Returns nil if VALUE is \"NONE\", otherwise returns VALUE unchanged."
  (if (and value (string-equal value "NONE"))
      nil
    value))

2. vimco-process-attributes

Processes attribute strings into a list of face attributes:

(defun vimco-process-attributes (attributes-string)
  "Process ATTRIBUTES-STRING into a list of face attributes.

Splits ATTRIBUTES-STRING by commas, looks up each attribute in
`vimco-attribute-alist', and combines them into a single list."
  (when attributes-string
    (let (new-attributes)
      (mapc
       (lambda (attribute)
         (setq attribute
               (cdr (assoc attribute vimco-attribute-alist)))
         (when attribute
           (setq new-attributes
                 (append attribute new-attributes))))
       (split-string attributes-string "," t))
      new-attributes)))

3. vimco-format-color-property

Formats color properties into plist entries:

(defun vimco-format-color-property (property-name value)
  "Format a color property into a plist entry.

PROPERTY-NAME should be a symbol like 'foreground or 'background.
VALUE should be a color string. Returns a list like (:foreground VALUE)
or nil if VALUE is nil."
  (when value
    `(,(intern (format ":%s" property-name)) ,value)))

4. vimco-build-face-attributes

Combines all components into a face attributes plist:

(defun vimco-build-face-attributes (foreground background attributes)
  "Build a face attributes plist from FOREGROUND, BACKGROUND, and ATTRIBUTES.

Combines the three components into a single plist suitable for face definitions."
  (let (face-attributes)
    (mapc
     (lambda (attribute)
       (when attribute
         (setq face-attributes
               (append face-attributes attribute))))
     (list attributes background foreground))
    face-attributes))

5. vimco-create-face-definitions

Creates the final face definition forms:

(defun vimco-create-face-definitions (face-names face-attributes)
  "Create face definitions for FACE-NAMES with FACE-ATTRIBUTES.

Returns a list of face definition forms suitable for `custom-theme-set-faces'."
  (let (faces)
    (mapc
     (lambda (face)
       (push
        `'(,face ((((class color) (min-colors 89))
                   (,@face-attributes))))
        faces))
     face-names)
    faces))

6. Updated vimco-line-to-faces

Now clearly shows the transformation pipeline:

(defun vimco-line-to-faces (line)
  "Transforms LINE into a list of face definitions.

LINE should be an alist returned by `vimco-parse-line'."
  (cl-flet ((get-prop (prop) (cdr (assoc prop line))))
    (let ((face-names (cdr (assoc (caar line) vimco-face-name-alist))))
      (when face-names
        (let* ((foreground (vimco-normalize-property-value (get-prop "guifg")))
               (background (vimco-normalize-property-value (get-prop "guibg")))
               (attributes (vimco-normalize-property-value
                           (or (get-prop "gui")
                               (get-prop "term")
                               (get-prop "cterm"))))
               (processed-attributes (vimco-process-attributes attributes))
               (formatted-foreground (vimco-format-color-property 'foreground foreground))
               (formatted-background (vimco-format-color-property 'background background))
               (face-attributes (vimco-build-face-attributes
                                formatted-foreground
                                formatted-background
                                processed-attributes)))
          (vimco-create-face-definitions face-names face-attributes))))))

Benefits

  • Single Responsibility Principle: Each function now has one clear, focused purpose
  • Improved Readability: The logic is easier to follow and understand at a glance
  • Better Maintainability: Changes to one aspect (e.g., word parsing, attribute processing) don't affect others
  • Reusability: Helper functions can be used elsewhere if needed in the future
  • Better Documentation: Each function has its own clear, specific docstring
  • Consistent Style: Both refactorings follow the same pattern of breaking complex functions into focused helpers

Functional Equivalence

The refactored code produces exactly the same output as the original implementation. This is purely a code organization improvement with no changes to the API or behavior.

Fixes the issue requesting to split up vimco-parse-line and vimco-line-to-faces into smaller functions for better readability.

Original prompt

Split up vimco-parse-line into smaller functions so the code becomes a bit more readable.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

Copilot avatar Oct 09 '25 08:10 Copilot