Skip to content

Conversation

@tagliala
Copy link
Member

@tagliala tagliala commented Jul 7, 2025

  • Use manual fix for - 1, by using ... instead of ..
  • Use automatic fix for + 1

@tagliala tagliala requested a review from Copilot July 7, 2025 10:15
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR addresses new Lint/AmbiguousRange offenses by replacing inclusive ranges with exclusive ones in string slicing.

  • Replace 0..last_slash_pos - 1 with 0...last_slash_pos in split_path
  • Update endless ranges to avoid ambiguous syntax in both split_path and sanitize_filename
Comments suppressed due to low confidence (2)

lib/sharepoint/client.rb:632

  • Add a unit test for split_path when file_path contains no / (so last_slash_pos is nil) to ensure this new exclusive range usage handles that edge case without raising an error.
        path: file_path[0...last_slash_pos],

lib/sharepoint/client.rb:743

  • [nitpick] Consider adding tests for sanitize_filename boundary conditions (e.g., filenames without extensions, maximum lengths, or dot positions) to validate the new exclusive-end slicing logic behaves as expected.
          sanitized_filename = sanitized_filename[0..upper_bound] + sanitized_filename[dot_index...sanitized_filename.length]

No functional change is intended, as these adjustments are semantically
equivalent in this context.

- Manually replaced ambiguous inclusive ranges (`..`) with exclusive
  ranges (`...`) for improved code clarity
- Applied RuboCop's automatic correction to refactor ranges with
  expressions like `[(start + 1)..end]`
@tagliala tagliala force-pushed the chore/update-rubocop branch from 1f72e5e to f343f24 Compare July 7, 2025 10:31
@tagliala tagliala merged commit 00954e2 into master Jul 7, 2025
7 checks passed
@tagliala tagliala deleted the chore/update-rubocop branch July 7, 2025 10:32
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.

2 participants