Skip to content

Add editor encoding api#177434

Closed
Tarrowren wants to merge 1 commit into
microsoft:mainfrom
Tarrowren:editor-encoding
Closed

Add editor encoding api#177434
Tarrowren wants to merge 1 commit into
microsoft:mainfrom
Tarrowren:editor-encoding

Conversation

@Tarrowren
Copy link
Copy Markdown

@Tarrowren Tarrowren commented Mar 17, 2023

see #824

@alexr00 alexr00 assigned alexdima and unassigned alexr00 Mar 17, 2023
@alexdima alexdima assigned bpasero and unassigned alexdima Mar 20, 2023
@bpasero bpasero added this to the Backlog milestone Mar 20, 2023
@helmut-hackl helmut-hackl mentioned this pull request Apr 11, 2023
Closed
Tosox

This comment was marked as outdated.

Sign in to view

@angelozerr
Copy link
Copy Markdown
Contributor

angelozerr commented Nov 16, 2023

Is there any chance to merge this PR?

@lygstate
Copy link
Copy Markdown

lygstate commented Dec 5, 2023

ping for merge

@lygstate lygstate mentioned this pull request Dec 5, 2023
Closed
1 task
lygstate
lygstate reviewed Dec 7, 2023
View reviewed changes
Comment thread src/vs/workbench/api/common/extHostTextEditor.ts Outdated
lygstate
lygstate reviewed Dec 7, 2023
View reviewed changes
Comment thread src/vscode-dts/vscode.proposed.editorEncoding.d.ts Outdated
@Tosox
Copy link
Copy Markdown

Tosox commented Jan 22, 2024

Is there any chance to merge this PR?

@denniskp
Copy link
Copy Markdown

denniskp commented Feb 20, 2024

Bump pull request

@feed3r
Copy link
Copy Markdown

feed3r commented Apr 16, 2024

We also asked for the very same thing and proposed to implement a PR like this here

It would be really good to know if this has some possibility to be merged or if there is any other reason to not merge it.

@billgan1024
Copy link
Copy Markdown

billgan1024 commented May 14, 2024

ping for merge

Tosox
Tosox reviewed Jun 27, 2024
View reviewed changes
Comment thread src/vs/workbench/services/extensions/common/extensionsApiProposals.ts Outdated
Comment thread src/vs/workbench/api/browser/mainThreadEditors.ts Outdated
@Tosox
Copy link
Copy Markdown

Tosox commented Jul 5, 2024

Nice to see you still maintaining this pr. I hope someone will finally merge this soon.

@leaumar
Copy link
Copy Markdown

leaumar commented Oct 25, 2024

Is this PR actually blocked by anything meaningful or are we just waiting for the right persons to stumble upon it by chance? This PR is needed for full editorconfig support, which I would hope the entire vscode team agrees is a good feature (or rather, an embarassing one to say is incomplete). There's barely any content to review too, it looks so simple it's probably perfectly good to merge as-is.

@leaumar
Copy link
Copy Markdown

leaumar commented Oct 28, 2024

@bpasero could you please push this forward by merging or assigning to someone? This would finally implement full .editorconfig support for all the people waiting on it (which is a lot more than the handful who've bothered to find this issue and vote on it). Thanks

@vivodi
Copy link
Copy Markdown

vivodi commented Dec 29, 2024

Is this still relevant?

@leaumar
Copy link
Copy Markdown

leaumar commented Dec 29, 2024

Is this still relevant?

Yes, very, lots of people are eagerly awaiting the merge

@yhatt yhatt mentioned this pull request Jan 20, 2025
Closed
@bpasero bpasero linked an issue Feb 7, 2025 that may be closed by this pull request
Provide encoding-related APIs for editor extensions #824
Closed
bpasero pushed a commit that referenced this pull request Feb 7, 2025
changes from #177434 with latest main
a97e187
@bpasero
Copy link
Copy Markdown
Contributor

bpasero commented Feb 7, 2025

@Tarrowren this change is heavily behind main, I tried to merge myself but miss the permissions to push back to your branch. I have https://github.com/microsoft/vscode/tree/ben/editor-encoding-api-fork with that work, how do you want to proceed, in here or in a new PR from https://github.com/microsoft/vscode/tree/ben/editor-encoding-api-fork ?

@bpasero bpasero added the info-needed Issue requires more information from poster label Feb 7, 2025
bpasero
bpasero suggested changes Feb 7, 2025
View reviewed changes
Copy link
Copy Markdown
Contributor

@bpasero bpasero left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@Tarrowren On a high level, the better location for encoding concerns is actually TextDocument. Because that maps to the underlying text model where the encoding information is at and where it can be changed. A text editor is never the owner of the documents encoding. This is the first change I am asking to do.

As for the API itself, encoding should be a readonly encoding: string that lives on TextDocument and can be received without awaiting a Promise. Subsequently, I would suggest a encode(encoding: string): Promise<void> and decode(encoding: string): Promise<void> method on TextDocument for the remainder.

Does that sound reasonable? Please let me know if you are willing to continue working on this PR given this feedback.

@yy0931 yy0931 mentioned this pull request Feb 9, 2025
Closed
@bpasero
Copy link
Copy Markdown
Contributor

bpasero commented Feb 13, 2025

Thus this continues in #239961

@bpasero bpasero closed this Feb 13, 2025
@Keyaku Keyaku mentioned this pull request Feb 25, 2025
Closed
@vs-code-engineering vs-code-engineering Bot locked and limited conversation to collaborators Mar 30, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

3 more reviewers

@lygstate lygstate lygstate left review comments

@Tosox Tosox Tosox approved these changes

@bpasero bpasero bpasero requested changes

Reviewers whose approvals may not affect merge requirements

Assignees

@bpasero bpasero

Labels

info-needed Issue requires more information from poster

Projects

None yet

Milestone

Backlog

Development

Successfully merging this pull request may close these issues.

Provide encoding-related APIs for editor extensions