eclipse-che / che

Kubernetes based Cloud Development Environments for Enterprise Teams
http://eclipse.org/che
Eclipse Public License 2.0
6.99k stars 1.19k forks source link

che-theia should have autosave activated by default #13740

Closed sunix closed 5 years ago

sunix commented 5 years ago

che-theia should have autosave activated by default

slemeur commented 5 years ago

Would you enabled it for all files and all stacks?

benoitf commented 5 years ago

@slemeur : you don't want to use a global flag ?

slemeur commented 5 years ago

Ideally, As a user I'd like to be able to configure the autosave behavior in my user preferences. But then, I'd like to be able to tune that on a per workspace basis too.

Do we have a mechanism to do that? (If not, I'll create an epic)

benoitf commented 5 years ago

I think the idea is to have autosave enabled by default but as any preference, you can customise in your IDE preferences the behaviour (the same behaviour we had before theia changed default)

mmorhun commented 5 years ago

Now we have synchronization mechanism as was requested before: if user changes a Theia preference in a workspace it will update defaults and the changes will be applied to other workspaces on their restart.

slemeur commented 5 years ago

there are certain preferences which would be nice to apply to all workspaces and there are certain other preferences which should stay specific to a single workspace.

Are you saying that, today, all preferences are spread across all user's workspaces?

mmorhun commented 5 years ago

All Theia preferences from user scope are shared between all workspaces (actually it is json file). If user wants to override something it could be easily done via Theia workspace preferences (which has higher priority and can override value from user preferences).

sunix commented 5 years ago

could we just have it by default ? what is the point not having this by default ?

evidolob commented 5 years ago

@slemeur Can you provide your vision on this issue? Should we work on it?

slemeur commented 5 years ago

https://github.com/theia-ide/theia/issues/4776#issuecomment-478892788

sunix commented 5 years ago

@slemeur @tsmaeder @nickboldt @l0rd @rhopp for GA ?

l0rd commented 5 years ago

@evidolob both @slemeur and myself think that we should have autosave activated by default on che-theia. Now there a couple of things that need to be analyzed to understand what is the milestion of this issue:

evidolob commented 5 years ago

How much work it requires to switch back che-theia to autosave?

It a matter of changing default settings(false-> true), but, AFAIK, Theia doesn't have API to override the default values. cc @akurinnoy

Is there a risk of unexpected side-effects?

No

In other words: upstream theia choice to disable autosave was related to a better/safer behavior or only for VSCode/Theia parity ?

Yes, it's pretty clear that the only reason is VSCode compatibility: https://github.com/theia-ide/theia/issues/4776

benoitf commented 5 years ago

override is in package.json https://github.com/theia-ide/theia/commit/9eb7f4e4fb38268f207ab7f8bd7adea45c6a97ed see chagelog

evidolob commented 5 years ago

Then, OK. @l0rd If you want to have this done for GA, please add proper labels.

tsmaeder commented 5 years ago

Is there a real reason to enable autosave by default except "it's what Che 6 did"? It seems a pretty rare default as far as IDE's go. Personally, I don't like the behaviour.

sunix commented 5 years ago

Is there a real reason to enable autosave by default except "it's what Che 6 did"? It seems a pretty rare default as far as IDE's go. Personally, I don't like the behaviour.

Most of the developers are expecting auto save and like the behaviour. We can make any survey any time, it will have the same result: https://twitter.com/sunsengdavidtan/status/1131142032582356993

Also this is a behaviour that we have in most web app: gdoc, gmail, etc ...

tsmaeder commented 5 years ago

But we're not writing an Web App, we're writing an IDE and IDE's don't typically have this behaviour. Also: not to cast aspersions on your twitter followers, but I hardly think that's a representative sample.

sunix commented 5 years ago

But we're not writing an Web App, we're writing an IDE and IDE's don't typically have this behaviour. Also: not to cast aspersions on your twitter followers, but I hardly think that's a representative sample.

... Google was writting a word processor and word processors typically didn't have this behaviour. Google was writting a email client and email clients typically didn't have this behaviour

Not representative ? at least it is more representative than your own opinion.

Let's close this debate as it's been decided by our decision makers to do it. By the way this would be the default option it doesn't mean you could not disabling it.

l0rd commented 5 years ago

No discussion has ever happened to disable autosave on Che. It has been disabled a couple of months ago without any notice or approval. That's something that we need to fix.

If a proper discussion happens in the future I am +1 to disable autosave but for the time being let's be consistent with previous version of Che.

slemeur commented 5 years ago

We are doing cloud tools since quite a lot of time and we learnt from users that there is one thing they hate: loosing their datas. That is a big concern regarding how users will fell confident to use our tool.

As such, it's reasonable to turn on auto-save by default to avoid loosing source code changes. If the user doesn't like this behavior, he'll go into his preferences and explicitly change the preference and agreed with/understand the risks.

sunix commented 5 years ago

https://github.com/eclipse/che-theia/pull/342 is not merged yet

akurinnoy commented 5 years ago

https://github.com/eclipse/che-theia/pull/342 is merged