Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mac case insensitivity must be addressed #92

Closed
dalehenrich opened this issue Jul 18, 2013 · 11 comments
Closed

Mac case insensitivity must be addressed #92

dalehenrich opened this issue Jul 18, 2013 · 11 comments
Milestone

Comments

@dalehenrich
Copy link
Owner

Just hit this working on the GLASS project ... since Mac is lowest common denominator, we have to bit the bullet ... (I hate it) ... will have to mangle the filenames of methods that have embedded capitalization ... this needs to be posted as a Cypress issue as well ... assuming that leading character in method name is not capitalized only need to come up with a scheme for making camel cased methods unique...

@martinmcclure
Copy link

I've seen a number of cases where the first character in a method name is capitalized, and there is a method with the same name except for that one capitalization (for instance, this pattern is fairly standard in ancient GBS code). So it'd be better to make case mangling work even for the first character, if possible.

@dalehenrich
Copy link
Owner Author

Thanks @martinmcclure, after an email convo with Richard and James, I think that we can use the ^ character to mark capitals so the following filenames would

  includesSubString:  maps to includes^Sub^String..st
  includesSubstring:  maps to includes^Substring..st

a leading ^ would work as well ...

Can you think of any reason the ^ would be a bad idea ... it's useful because the ^ is not a legal selector character.

James suggested doubling the _ character, so a single _ would make capitalized letters unique ...

@dalehenrich
Copy link
Owner Author

Hold on...we already use ^ when we replace special characters. The method + becomes ^plus.st and so on ...

@dalehenrich
Copy link
Owner Author

I'm leaning toward using double period as the capitalization escape:

  includes:substring: becomes  includes.substring..st
  includes:subString: becomes  includes.sub..String..st
  includesSubString: becomes  includes..Sub..String..st
  includesSubstring: becomes  includes..Substring..st
  includes:Substring: becomes includes...Substring..st
  Includes:Substring: becomes ..Includes...Substring..st

Not too bad. I already know that the absence of : in the filename is a bit confusing ... but if you've got a string of selectors like the above in the same class the .. actually makes the distinction between the selectors more obvious:)

@dalehenrich
Copy link
Owner Author

After further "conversations" with James and Richard covering the pros and cons of the above approach and additional approaches, I am now leaning towards an approach where we only invoke distinguishment when there is a case insensitive collision between two filenames.

When a collision occurs we tack on a double period and a sequence number so the above mappings become:

  includes:substring: becomes  includes.substring...1.st
  includes:subString: becomes  includes.subString...2.st
  includes:Substring: becomes includes.Substring...3.st
  Includes:Substring: becomes Includes.Substring...4.st
  includesSubString: becomes  includesSubString...1.st
  includesSubstring: becomes  includesSubstring...2.st

The ..X postfix guarantees that the filenames will be unique regardless of whether or not the underlying filesystem is case insensitive or not.

It's a bit of work to determine when you are exposed, but I think the advantage of leaving the filenames as unmangled as possible outweighs the computation costs....

dalehenrich pushed a commit that referenced this issue Jul 19, 2013
@dalehenrich
Copy link
Owner Author

tests green ... didn't break any existing behavior (covered by tests)...time for some new tests

dalehenrich pushed a commit that referenced this issue Jul 19, 2013
@dalehenrich
Copy link
Owner Author

test case and test repo added ... green tests ... need to distribute changes to other platforms ...

dalehenrich pushed a commit that referenced this issue Jul 23, 2013
(cherry picked from commit 24c63a7)

Issue #92 bugfix

Conflicts:

	repository/MonticelloFileTree-Core.package/monticello.meta/version
	repository/MonticelloFileTree-Tests.package/monticello.meta/version
dalehenrich pushed a commit that referenced this issue Aug 8, 2013
(Issue #92 and Issue #93)

Conflicts:
	repository/MonticelloFileTree-Core.package/MCFileTreeStCypressReader.class/methodProperties.json
	repository/MonticelloFileTree-Core.package/monticello.meta/version
dalehenrich pushed a commit that referenced this issue Aug 9, 2013
(Issue #11, #92, #93, #101)

Conflicts:
	repository/MonticelloFileTree-Core.package/MCFileTreeStSnapshotWriter.class/instance/writeDefinitions..st
	repository/MonticelloFileTree-Core.package/monticello.meta/version
dalehenrich pushed a commit that referenced this issue Aug 9, 2013
(Issue #11, #92, #93, #101)

Conflicts:
	repository/MonticelloFileTree-Core.package/MCFileTreeStCypressReader.class/instance/loadVersionInfo.st
	repository/MonticelloFileTree-Core.package/MCFileTreeStCypressReader.class/methodProperties.json
	repository/MonticelloFileTree-Core.package/MCFileTreeStCypressWriter.class/instance/writeDefinitions..st
	repository/MonticelloFileTree-Core.package/MCFileTreeStCypressWriter.class/methodProperties.json
	repository/MonticelloFileTree-Core.package/MCFileTreeStSnapshotWriter.class/instance/writeDefinitions..st
	repository/MonticelloFileTree-Core.package/MCFileTreeStSnapshotWriter.class/instance/writeMethodHolderDefinitions.extension.to.do..st
	repository/MonticelloFileTree-Core.package/MCFileTreeStSnapshotWriter.class/methodProperties.json
	repository/MonticelloFileTree-Core.package/monticello.meta/version
	repository/MonticelloFileTree-Tests.package/monticello.meta/version
dalehenrich pushed a commit that referenced this issue Aug 9, 2013
(Issue #11, #92, #93, #101)

Conflicts:
	repository/MonticelloFileTree-Core.package/MCFileTreeStSnapshotWriter.class/instance/writeDefinitions..st
	repository/MonticelloFileTree-Core.package/MCFileTreeStSnapshotWriter.class/methodProperties.json
dalehenrich pushed a commit that referenced this issue Aug 9, 2013
(Issue #11, #92, #93, #101)

Conflicts:
	repository/MonticelloFileTree-Core.package/MCFileTreeStCypressWriter.class/methodProperties.json
	repository/MonticelloFileTree-Core.package/monticello.meta/version
	repository/MonticelloFileTree-Tests.package/monticello.meta/version
dalehenrich pushed a commit that referenced this issue Aug 9, 2013
(Issue #11, #92, #93, #101)

Conflicts:
	repository/MonticelloFileTree-Core.package/MCFileTreeStCypressWriter.class/methodProperties.json
	repository/MonticelloFileTree-Core.package/monticello.meta/version
	repository/MonticelloFileTree-Tests.package/monticello.meta/version
dalehenrich pushed a commit that referenced this issue Aug 9, 2013
(Issue #11, #92, #93, #101)

Conflicts:
	README.md
	repository/MonticelloFileTree-Core.package/MCFileTreeStCypressWriter.class/instance/writeDefinitions..st
	repository/MonticelloFileTree-Core.package/MCFileTreeStCypressWriter.class/methodProperties.json
	repository/MonticelloFileTree-Core.package/MCFileTreeStSnapshotWriter.class/instance/writeDefinitions..st
	repository/MonticelloFileTree-Core.package/MCFileTreeStSnapshotWriter.class/instance/writeMethodHolderDefinitions.extension.to.do..st
	repository/MonticelloFileTree-Core.package/MCFileTreeStSnapshotWriter.class/methodProperties.json
	repository/MonticelloFileTree-Core.package/monticello.meta/version
	repository/MonticelloFileTree-Tests.package/monticello.meta/version
dalehenrich pushed a commit that referenced this issue Aug 9, 2013
(Issue #11, #92, #93, #101)

Conflicts:
	README.md
	repository/MonticelloFileTree-Core.package/MCFileTreeStCypressReader.class/instance/loadVersionInfo.st
	repository/MonticelloFileTree-Core.package/MCFileTreeStCypressWriter.class/instance/writeDefinitions..st
	repository/MonticelloFileTree-Core.package/MCFileTreeStCypressWriter.class/methodProperties.json
	repository/MonticelloFileTree-Core.package/MCFileTreeStSnapshotWriter.class/instance/writeDefinitions..st
	repository/MonticelloFileTree-Core.package/MCFileTreeStSnapshotWriter.class/instance/writeMethodHolderDefinitions.extension.to.do..st
	repository/MonticelloFileTree-Core.package/MCFileTreeStSnapshotWriter.class/methodProperties.json
	repository/MonticelloFileTree-Core.package/monticello.meta/version
	repository/MonticelloFileTree-Tests.package/monticello.meta/version
@dalehenrich
Copy link
Owner Author

distributed to all platforms

@krono
Copy link
Collaborator

krono commented Aug 9, 2013

Dale, a small comment on that:
the method name is replicated inside the method itself, so we actually just have to make sure the method names are unique, no? So I would have proposed to

  • downcase all method names and
  • on name clash, just add charactest (like _ or .) until unique

Just my 2ct

@dalehenrich
Copy link
Owner Author

I decided that I didn't really want to mangle ALL of the method names when a very small percentage of the methods are going to be affected ... yeah we talked about a whole slew of algorithms .. one idea that we through around was to tack the hash of the method onto the end of each filename ... there are a lot of ways to do this and I picked one ...

----- Original Message -----

| From: "Tobias Pape" notifications@github.com
| To: "dalehenrich/filetree" filetree@noreply.github.com
| Cc: "Dale Henrichs" dale.henrichs@gemtalksystems.com
| Sent: Friday, August 9, 2013 1:27:34 AM
| Subject: Re: [filetree] Mac case insensitivity must be addressed
| (#92)

| Dale, a small comment on that:
| the method name is replicated inside the method itself, so we
| actually just have to make sure the method names are unique, no? So
| I would have proposed to

| * downcase all method names and
| * on name clash, just add charactest (like _ or .) until unique

| Just my 2ct
| —
| Reply to this email directly or view it on GitHub .

@krono
Copy link
Collaborator

krono commented Aug 9, 2013

👍 I like that.

dalehenrich pushed a commit that referenced this issue Aug 10, 2013
(Issue #11, #92, #97, #101)

Conflicts:
	repository/MonticelloFileTree-Core.package/MCFileTreeStCypressWriter.class/methodProperties.json
	repository/MonticelloFileTree-Core.package/monticello.meta/version
	repository/MonticelloFileTree-Tests.package/monticello.meta/version
dalehenrich pushed a commit that referenced this issue Aug 11, 2013
-fix Issue #11: Traits and Script support
- fix Issue #92: Mac case insensitivity `must` be addressed
- fix Issue #97: Load packages from repos with no monticello meta data
- fix Issue #101: Refactor MCFileTreeStCypressWriter>>writeDefinitions: to ease Issue #11 integration
for details: https://github.com/dalehenrich/filetree/issues?direction=desc&milestone=9&page=1&sort=updated&state=closed
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

No branches or pull requests

3 participants