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

[stable16] Change name from 'Text editor' to 'Plain text editor' to prevent confusion with 'Text' #169

Merged
merged 3 commits into from
Aug 8, 2019

Conversation

backportbot-nextcloud[bot]
Copy link

backport of #165

@jancborchardt
Copy link
Member

I’m a bit worried the "Edit in plain text editor" will be slightly long in the menu. Maybe we should use "Edit plain text"?

(But also, this will only show up when you have both the Text app and files_texteditor installed, right?)

@juliusknorr
Copy link
Member

I’m a bit worried the "Edit in plain text editor" will be slightly long in the menu. Maybe we should use "Edit plain text"?

The thing here is that the new editor also supports editing plain text. Did we decide yet if we ship the files_texteditor with 17? If not, it should be fine by default, since there is only text installed then.

(But also, this will only show up when you have both the Text app and files_texteditor installed, right?)

Correct.

@jancborchardt
Copy link
Member

Did we decide yet if we ship the files_texteditor with 17?

I guess not, since Text can do everything and more. @rullzer @MorrisJobke can you confirm?

@rullzer
Copy link
Member

rullzer commented Jul 16, 2019

Trouble is I think that even if we do not ship it it will still stay enabled on the users that upgrade.

So not shipping is fine but doesn't solve the issue currently.

@jancborchardt
Copy link
Member

Is there something that we can do about that? Either in server or in the Text app?

Cause having this extra menu entry for all existing users is quite a bit of a nuisance.

@juliusknorr
Copy link
Member

@rullzer Since the files_texteditor is not shipped anymore, shouldn't it be removed from the apps directory during the update?

@MorrisJobke
Copy link
Member

Trouble is I think that even if we do not ship it it will still stay enabled on the users that upgrade.

So not shipping is fine but doesn't solve the issue currently.

It should be removed by the updater, because it removes everything from the shipped.json. ;)

@rullzer
Copy link
Member

rullzer commented Jul 16, 2019

@MorrisJobke but files_texteditor is no longer shipper right...

@MorrisJobke
Copy link
Member

@MorrisJobke but files_texteditor is no longer shipper right...

That is the plan for 17, yes.

@rullzer
Copy link
Member

rullzer commented Jul 16, 2019

@MorrisJobke yes so if it is no longer shipped then it won't be in shipped.json and thus it won't be removed on upgrade. So it will just stay installed for users.

@MorrisJobke
Copy link
Member

@MorrisJobke yes so if it is no longer shipped then it won't be in shipped.json and thus it won't be removed on upgrade. So it will just stay installed for users.

It will be removed, because the old shipped.json is used to remove everything. Otherwise we would never be able to delete any file. 😉

@rullzer
Copy link
Member

rullzer commented Jul 16, 2019

aaaah
perfect then.

@MorrisJobke
Copy link
Member

aaaah
perfect then.

Seems to not work: https://drone.nextcloud.com/nextcloud/updater/181/11/2

2019-07-22T08:40:06+00:00 UnexpectedValueException: The files of the app "files_texteditor" were not correctly replaced before running the update

😢

@juliusknorr
Copy link
Member

juliusknorr commented Jul 22, 2019

Hm, ok I just checked the update and the folder was removed properly but the app is still marked as enabled in the appconfig table.

@MorrisJobke Should we add a repair step to disable it automatically or maybe just make sure to disable apps automatically when there is no app folder found?

@juliusknorr
Copy link
Member

Didn't we do the same with the user_external app? Was this an issue back then as well?

@MorrisJobke
Copy link
Member

Didn't we do the same with the user_external app? Was this an issue back then as well?

Good question. There we replaced it with a version from the App Store AFAIK.

@juliusknorr
Copy link
Member

Ah so the app was automatically updated since the app store had a newer version that was compatible with the new server release then?

@MorrisJobke
Copy link
Member

Ah so the app was automatically updated since the app store had a newer version that was compatible with the new server release then?

I think so

jancborchardt and others added 2 commits August 8, 2019 09:06
…usion with 'Text'

Signed-off-by: Julius Härtl <jus@bitgrid.net>
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@rullzer rullzer force-pushed the backport/165/stable16 branch from 3bcbc2d to 12c9684 Compare August 8, 2019 07:07
@georgehrke
Copy link
Member

/compile fixup /

Signed-off-by: Julius Härtl <jus@bitgrid.net>
@rullzer rullzer force-pushed the backport/165/stable16 branch from 4c5d847 to f6191a3 Compare August 8, 2019 17:19
@rullzer rullzer merged commit 2656a67 into stable16 Aug 8, 2019
@delete-merged-branch delete-merged-branch bot deleted the backport/165/stable16 branch August 8, 2019 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants