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

Add event information in onDidSaveTextDocument #5305

Closed
SamVerschueren opened this issue Apr 15, 2016 · 9 comments
Closed

Add event information in onDidSaveTextDocument #5305

SamVerschueren opened this issue Apr 15, 2016 · 9 comments
Assignees
Labels

Comments

@SamVerschueren
Copy link
Contributor

  • VSCode Version: 1.0.0
  • OS Version: Mac OSX

It would be nice if extensions are able to distinguish the autoSave event from a manual save. We need this information in our EditorConfig extension.

// @bpasero @jrieken

@jrieken
Copy link
Member

jrieken commented Apr 15, 2016

This is a duplicate of #239

@jrieken jrieken closed this as completed Apr 15, 2016
@jrieken jrieken added *duplicate Issue identified as a duplicate of another issue(s) and removed *duplicate Issue identified as a duplicate of another issue(s) labels Apr 29, 2016
@SamVerschueren
Copy link
Contributor Author

Is this actually really a duplicate of that issue? That issue is about adding new event types like onPreSaveTextDocument. This is more about adding extra information to the event being dispatched.

As you said before, it's quite difficult to change that without breaking the API. Currently only the TextDocument is being dispatched. It would be possible to attach an event type to the TextDocument but that's not perfect. It would make more sense if it was an event object, something like

interface EventObject {
    document: TextDocument,
    action: string
}

Where action is something like autoSave or save. But that will off course break the entire API.

@jrieken jrieken removed the *duplicate Issue identified as a duplicate of another issue(s) label May 11, 2016
@jrieken jrieken reopened this May 11, 2016
@jrieken
Copy link
Member

jrieken commented May 11, 2016

Sorry, I was wrong here... yeah, it's on_Will_ and on_Did_... Will be hard to make it work in a nice way...

@NinoFloris
Copy link

How is everything doing here?

@jrieken
Copy link
Member

jrieken commented Sep 23, 2016

Since we are facing API breakage with this, it might help if we expose the required data in the onWillSaveTextDocument-event. The save reason is something we try to squeeze in for Sep (#239 (comment))

@SamVerschueren
Copy link
Contributor Author

SamVerschueren commented Sep 23, 2016

@jrieken Would this still be considered in VS Code 2.0.0 or something? It feels a little like a design flaw to pass in the TextDocument directly as event instead of an event object with the property document and some extra properties like the save reason? I'm ok if not, it's the call of the core team, just want to make sure you guys did consider it :).

@jrieken
Copy link
Member

jrieken commented Sep 26, 2016

It clearly is a design flaw and could not come up with a back-wards compatible way to introducing such an event unless its like SaveEvent extends TextDocument which is a little weird. Maybe some TS/JS trickery ala Event<TextDocument & SaveEvent>

@SamVerschueren
Copy link
Contributor Author

If VS Code 2.x means breaking changes (major semver change), it might be possible to push this through. BUT, I understand that this is very hard to do because that means that every extension relying on this behaviour (which is probably 99% of all the extensions), will not work on VS Code 2.x.

Maybe some TS/JS trickery ala Event<TextDocument & SaveEvent>

Yes, if that would be backwards compatible AND if somehow, we could force in the type definitions, that in the next version the object returned is an Event object with properties document and some extra's (which under the hood actually still is a TextDocument with just some extra properties). Then, one day, we might be able to get rid of the hack.

I think it might be worth exploring as this might allow to add new features more easily in the future.

@jrieken
Copy link
Member

jrieken commented Apr 11, 2017

Closing this because it would require API breakage and because the information asked for is available in the onWillSave-event

@jrieken jrieken closed this as completed Apr 11, 2017
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants