rubberduck-vba / Rubberduck

Every programmer needs a rubberduck. COM add-in for the VBA & VB6 IDE (VBE).
https://rubberduckvba.com
GNU General Public License v3.0
1.91k stars 299 forks source link

Rubberduck doesn't allow renaming a module name great than 31 characters. #6132

Open MarkJohnstoneGitHub opened 1 year ago

MarkJohnstoneGitHub commented 1 year ago

Version 2.5.2.5906 OS: Microsoft Windows NT 10.0.19045.0, x64 Host Product: Microsoft Office 2016 x64 Host Version: 16.0.5317.1000 Host Executable: MSACCESS.EXE

Rubberduck doesn't allow renaming a module name great than 31 characters.
However I can create modules with names greater than 31 characters which RD doesn't allow.

Steps to reproduce the behavior:

  1. Go to Code Explorer
  2. Right Click on a module and select rename.

Expected behavior Able to rename a module greater than 31 characters.

image

retailcoder commented 1 year ago

Removing the [bug] tag since this is working as intended as per documented language specifications... however if there's a longer maximum length in Access, Rubberduck would certainly be enhanced by using Access-specific maximum length for module names when it knows it's hosted in Access (note: "library-specific" tag is used here to mean "host-specific").

retailcoder commented 1 year ago

Note: the disabled state in the screenshot is due to the new name matching the old name 😉

BZngr commented 1 year ago

Related #4819. Another Refactoring-Rename validation issue specific to MS Access.

MarkJohnstoneGitHub commented 1 year ago

I must test in Excel if an issue and if so do some renaming. Ok In Excel limitation of 31 characters for module names so must be just Access. Weird that allow that inconsisency. Looks like best practice keep to maximum of 31 characters. I'd recommned adding in a code inspection warning if module names exceed 31 characters for MS-Access.

retailcoder commented 1 year ago

I like the solution proposed by @bclothier in the linked issue; we lift the cap and give the rename a try - still host-agnostic, with the benefit of allowing any other host-specific name validation we don't know of. Plus, it introduces an opportunity for a new NonPortableIdentifier inspection that would flag identifier names that aren't portable to other hosts.

BZngr commented 1 year ago

Refactor-Rename is also used as a RefactoringAction within other Refactorings and QuickFixes (e.g., EncapsulateField) where there is the possibility of changing several names in a single action. Allowing Rename-Refactoring to give the rename a try with the possibility of failure would need to be accounted for in those refactorings as well.

MarkJohnstoneGitHub commented 1 year ago

Note: the disabled state in the screenshot is due to the new name matching the old name 😉 @retailcoder Actually that's doesn't appear to be the behaviour of the disable state when renaming a module. It's only disabled when greater than 31 characters or an invalid character for a module name. 😉

For now on I'm using RD renaming to make sure it's less than 31 characters so no compatibility issues with importing into MS-Excel etc.