Editor preview support for external renderers

[brechtvl]

Remove [repository.editor] PREVIEWABLE_FILE_MODES setting that seemed like it was intended to support this but did not work. Instead, whenever viewing a file shows a preview, also have a Preview tab in the file editor.

Add new /markup web and API endpoints with comment, gfm, markdown and new file mode that uses a file path to determine the renderer.

Remove /markdown web endpoint but keep the API for backwards and GitHub compatibility.

⚠️ BREAKING ⚠️

The [repository.editor] PREVIEWABLE_FILE_MODES setting was removed. This setting served no practical purpose and was not working correctly. Instead a preview tab is always shown in the file editor when supported.

[brechtvl]

Example:

[brechtvl]

The /markdown and MarkdownOption names aren't accurate anymore,. Should I create a new /markup and MarkupOption, and keep the old ones around for backwards compatibility? Rename and break compatibility? Leave it as is?

[silverwind]

Yeah, rename and update docs, and keep the old ones as compat alias. Ideally we should deprecate but I don't think we have a system in place for this.

[lunny]

For internal web routers, I think we can just use /markup replace /markdown and don't care the change of routers.

For API endpoints, we can keep both endpoints at the moment. /markdown will be kept as before but not external renderers. And /markup could include all /markdown

[lunny]

"file" mode looks like not work. I put the xxxx of [markup.xxxx] in PREVIEWABLE_FILE_MODES, then it works.

[brechtvl]

I'm not sure what is not working exactly? For me the /markup API in mode file works for all [markup. xxxx].

[repository.editor] PREVIEWABLE_FILE_MODES only affects if the preview tab is displayed in the editor or not. The only thing setting.Repository.Editor.PreviewableFileModes is used for in the code is to tell the editor javascript code which file extensions to show a preview tab for.

[lunny]

I'm not sure what is not working exactly? For me the /markup API in mode file works for all [markup. xxxx].

[repository.editor] PREVIEWABLE_FILE_MODES only affects if the preview tab is displayed in the editor or not. The only thing setting.Repository.Editor.PreviewableFileModes is used for in the code is to tell the editor javascript code which file extensions to show a preview tab for.

This is my app.ini

[repository.editor]
PREVIEWABLE_FILE_MODES = comment, gfm, markdown, file

[markup.asciidoc]
ENABLED         = true
FILE_EXTENSIONS = .adoc,.asciidoc
RENDER_COMMAND  = asciidoctor -s -a showtitle --out-file=- -
; Input is not a standard input but a file
IS_INPUT_FILE   = false

And when I edit a file end with .asciidoc on the UI, the preview tab will not be displayed.

[brechtvl]

Ah, it has to be like this with the current implementation:

PREVIEWABLE_FILE_MODES = markdown, asciidoc

That's how I understood it was meant to work. Though I'm unsure why this setting exists at all really, why the editor doesn't just display a preview for every extension it knows, same as when just viewing the file.

[brechtvl]

I guess this was meant to map to the mode for /markup? But if so I'm still unsure what the purpose of this is. If we want to control if a file is displayed with markdown or gfm for example (not sure why), an editor setting still seems like the wrong place for it.

[silverwind]

why the editor doesn't just display a preview for every extension it knows, same as when just viewing the file.

Isn't preview always server-rendered, e.g. backed by a API that is exposed by the configurable markup renderers? At least I don't think we use any kind of preview functionality of Monaco itself, and I'm not aware that it has any.

[lunny]

Ah, it has to be like this with the current implementation:

PREVIEWABLE_FILE_MODES = markdown, asciidoc

That's how I understood it was meant to work. Though I'm unsure why this setting exists at all really, why the editor doesn't just display a preview for every extension it knows, same as when just viewing the file.

This does work. I made a mistake to let PREVIEWABLE_FILE_MODES = file which should not be supported.
And for the default value, maybe we can have a break change. Defaultly all files markup supported should be allowed to preview except PREVIEWABLE_FILE_MODES given a special value. Currently it's markdown, maybe we can change it to empty.

[brechtvl]

Isn't preview always server-rendered, e.g. backed by a API that is exposed by the configurable markup renderers? At least I don't think we use any kind of preview functionality of Monaco itself, and I'm not aware that it has any.

It is always server-rendered.

And for the default value, maybe we can have a break change. Defaultly all files markup supported should be allowed to preview except PREVIEWABLE_FILE_MODES given a special value. Currently it's markdown, maybe we can change it to empty.

Could we remove PREVIEWABLE_FILE_MODESentirely? That's barely a breaking change, more of a bugfix in my opinion.

I can't think of a scenario where you want to have a preview when viewing the file, but then when you click edit not have a Preview tab available. It seems useless to me, and that's really the only thing this setting does.

[lunny]

Isn't preview always server-rendered, e.g. backed by a API that is exposed by the configurable markup renderers? At least I don't think we use any kind of preview functionality of Monaco itself, and I'm not aware that it has any.

It is always server-rendered.

And for the default value, maybe we can have a break change. Defaultly all files markup supported should be allowed to preview except PREVIEWABLE_FILE_MODES given a special value. Currently it's markdown, maybe we can change it to empty.

Could we remove PREVIEWABLE_FILE_MODESentirely? That's barely a breaking change, more of a bugfix in my opinion.

I can't think of a scenario where you want to have a preview when viewing the file, but then when you click edit not have a Preview tab available. It seems useless to me, and that's really the only thing this setting does.

I'm OK to remove the configuration item.

[brechtvl]

I did implement what was suggested in that comment.

Please see the PR description and other comments for details on what the breaking change is, and why I think it's barely a breaking change at all and more of a bugfix.

[silverwind]

Removing a broken setting is barely breaking, I agree. But it should be mentioned at least so people can remove it from their configs.