Skip to content

Add an explicit note about drive letter colons - #1786

Merged
Dirk Bäumer (dbaeumer) merged 4 commits into
microsoft:gh-pagesfrom
DanTup:note-escaped-colons
Oct 19, 2023
Merged

Dirk Bäumer (dbaeumer) merged 4 commits into
microsoft:gh-pagesfrom
DanTup:note-escaped-colons

Conversation

@DanTup

Copy link
Copy Markdown
Contributor

Comment thread _includes/types/uri.md Outdated

Many of the interfaces contain fields that correspond to the URI of a document. For clarity, the type of such a field is declared as a `DocumentUri`. Over the wire, it will still be transferred as a string, but this guarantees that the contents of that string can be parsed as a valid URI.

Care should be taken to handle encoded in URIs. Some clients (such as VS Code) may encode colons in drive letters while others do not. The following URIs should be considered equivalent and are both valid for clients and servers to use:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would actually phrase this more general saying that servers should handle all valid encoded URIs. For example clients might use either file:///c:/project/readme.md or file:///c%3A/project/readme.md to denote a file with a drive letter under Windows. However clients should be consistent in how they encode characters in URIs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

However clients should be consistent in how they encode characters in URIs.

I don't like the word should 😁

I've pushed some tweaks - how's this? (I explicitly noted that one party should not assume the other party will use the same encoding, because I think it has to be that way - perhaps I should have noted the same about drive letter casing?)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What could also be problematic and where care should be taken is when clients send symlinked file URIs. Or dealing with case-insensitive filenames. I would argue a server cannot rely on the string-representation of file URIs.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yep, there are definitely more edge cases than this, but this one is simple (it's just encoding/decoding) and already caused me a problem. It's much less clear to me what the spec should say about symlinks or casing so I think they could be handled in their own PRs (assuming some rules can be agreed!).

@DanTup

Copy link
Copy Markdown
Contributor Author

Dirk Bäumer (@dbaeumer) I pushed some tweaks based on your comments above. Would you like anything else changed?

Comment thread _includes/types/uri.md
Comment thread _includes/types/uri.md
@dbaeumer
Dirk Bäumer (dbaeumer) enabled auto-merge (squash) October 19, 2023 06:48
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.

5 participants