Skip to content

Add text/asciidoc mimetype - #3535

Merged
max-nextcloud merged 3 commits into
nextcloud:masterfrom
bjornfor:add-asciidoc-mimetype
Dec 8, 2022
Merged

max-nextcloud merged 3 commits into
nextcloud:masterfrom
bjornfor:add-asciidoc-mimetype

Conversation

@bjornfor

@bjornfor bjornfor commented Dec 7, 2022 •

Copy link
Copy Markdown
Contributor

馃摑 Summary

This allows editing AsciiDoc files, as plain text.

This change depends on
nextcloud/server@582f07c
("Add text/asciidoc mimetype").

Signed-off-by: Bj酶rn Forsman [email protected]

Disclaimer: this is a trivial change, but I haven't tested (I don't know how).
I based this on
#394 (comment)
It has a test now.

馃毀 TODO

  • ...

馃弫 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests
  • Documentation (README or documentation) has been updated or is not required

@max-nextcloud max-nextcloud mentioned this pull request Dec 8, 2022
4 tasks done
@max-nextcloud

max-nextcloud commented Dec 8, 2022 •

Copy link
Copy Markdown
Collaborator

Hi @bjornfor,

Thanks a lot! That seems like a useful addition. Code also looks good 馃槈 .

I added a test for two mimetypes we already support here: #3540.
Would that work as a basis for adding tests for you?

I'd be curious to see if you can add a test with this. If you get stuck or miss some documentation please let me know so we can make this work more smoothly.

@bjornfor

bjornfor commented Dec 8, 2022

Copy link
Copy Markdown
Contributor Author

Hi @max-nextcloud, thanks, I can probably make a copy of the plain text test, without fully understanding it 馃槃

This allows editing AsciiDoc files, as plain text.

This change depends on
nextcloud/server@582f07c
("Add text/asciidoc mimetype").

Signed-off-by: Bj酶rn Forsman <[email protected]>
@bjornfor
bjornfor force-pushed the add-asciidoc-mimetype branch from 7aed707 to 89ceaae Compare December 8, 2022 16:48
@bjornfor

bjornfor commented Dec 8, 2022

Copy link
Copy Markdown
Contributor Author

@max-nextcloud: I rebased and added a test, but I think cy.getContent().find('pre').should('exist') part isn't quite right.

So find() looks for HTML tags (right?), and in the "empty.md" test you added I think the test just checks that the overall HTML page contains a pre element. In the "markdown" test, the code checks for a h2 tag that comes from converting markdown to HTML. But for this AsciiDoc code, we don't get any special markup (it's treated as plain text), but we would perhaps like to check that the text content got rendered?

I guess I'd like to say cy.getContent().SOMETHING_HERE.should('contain', 'Hello world'), but don't know what SOMETHING_HERE should be.

@bjornfor

bjornfor commented Dec 8, 2022

Copy link
Copy Markdown
Contributor Author

I guess I'd like to say cy.getContent().SOMETHING_HERE.should('contain', 'Hello world'), but don't know what SOMETHING_HERE should be.

Does cy.getContent().find('pre').should('contain', 'Hello world') work?

@susnux

susnux commented Dec 8, 2022 •

Copy link
Copy Markdown
Contributor

The getContent() part restricts the following find to the editor content (whereas get() always searches the whole document).

For plaintext documents the editor just contains

<pre><code>
FILE CONTENT
</code></pre>

So this is working as expected.

@max-nextcloud
max-nextcloud self-requested a review December 8, 2022 18:12

@max-nextcloud max-nextcloud left a comment •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hey @bjornfor

Thanks for adding the test. Congratulations for figuring it out without understanding it.
馃攽 achievement unlocked 馃槈

Does cy.getContent().find('pre').should('contain', 'Hello world') work?

Yes - that should do the trick. As @susnux explained getContent only looks at the content of the editor.
Feel free to add the hello world check - test also seems okay for me as is because it tests that text is loading the file.

Your last commit seems to lack a signoff. Other than that i think this is ready to go.

The failing node test is because the js still needs to be build here by commenting with /compile. It's a mess and we're working on fixing it. I can take care of that once you signed off the second commit.

@bjornfor
bjornfor force-pushed the add-asciidoc-mimetype branch from 89ceaae to 7ddfe07 Compare December 8, 2022 18:15
@bjornfor

bjornfor commented Dec 8, 2022

Copy link
Copy Markdown
Contributor Author

@susnux @max-nextcloud: Thanks. I added Signed-off-by: and update the test with cy.getContent().find('pre').should('contain', 'Hello world').

@max-nextcloud

Copy link
Copy Markdown
Collaborator

/compile

Signed-off-by: nextcloud-command <[email protected]>
@max-nextcloud

Copy link
Copy Markdown
Collaborator

CI failure is not related.

@max-nextcloud
max-nextcloud merged commit eea1ade into nextcloud:master Dec 8, 2022
@bjornfor
bjornfor deleted the add-asciidoc-mimetype branch December 8, 2022 20:05
@bjornfor

bjornfor commented Dec 8, 2022

Copy link
Copy Markdown
Contributor Author

@max-nextcloud: Thanks!

@mejo- mejo- mentioned this pull request Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants