Add the markdown entry
ClosedPublic

Authored by kqwyf on Aug 11 2018, 1:57 AM.

Details

Summary

Add the markdown entry to Cantor. Markdown support enables users to
write a document with runnable code in Markdown style. This
implementation supports LaTeX just like the text entry.

A third-party library Discount is linked in order to convert
markdown into HTML.

Update CMakeLists.txt

A screenshot for preview:

Diff Detail

Repository
R55 Cantor
Branch
feature (branched from master)
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 1892
Build 1910: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes
kqwyf added a comment.Aug 14 2018, 3:24 PM
  • remove the commented code
  • fix a bug that a command entry is appended when clicking a new markdown entry
kqwyf added a comment.Aug 14 2018, 3:39 PM

Thank you for your contribution @kqwyf. I built with Discount and it is working fine. I built without Discount, I could run Cantor correctly and the options to insert Markdown entries continues to show, but it is a problem with others entries options like LaTeX. Maybe it is time to some refactoring in this part.

For a first contribution it is very good, congrats. Stay with us to develop more features and fix bugs. :)

I will wait some hours for comments of the others reviewers, if they are nothing to ask I will push to you.

I think it a bit hard for me to refactor this part. I probably can't do it in this patch.

If I insert Markdown entry from context menu (this menu also have "Evaluate Worksheet" action) in the end of worksheet and click after it, a command entry added. Could anyone reproduce it?

I didn't notice it when I was testing. Thank you for reporting this bug. It is fixed.

@kqwyf Could you find Discount twice? Because I could found Discount only in first cmake run, and in seconds I have

-- The following OPTIONAL packages have not been found:

 * Discount
CMakeLists.txt
72–78

Maybe add set_package_properties like for LibSpectre?
Something like:

set_package_properties(Discount PROPERTIES DESCRIPTION "a C implementation of the Markdown markup language"
        URL "https://www.pell.portland.or.us/~orc/Code/discount/"
        TYPE OPTIONAL
        PURPOSE "Used for Markdown entries in Cantor")
kqwyf updated this revision to Diff 39734.Aug 14 2018, 4:07 PM
  • fix the bug that it won't find Discount when doing cmake twice.
  • add package properties for Discount
kqwyf added a comment.Aug 14 2018, 4:14 PM

@kqwyf Could you find Discount twice? Because I could found Discount only in first cmake run, and in seconds I have

-- The following OPTIONAL packages have not been found:

 * Discount

It seems not the package properties that cause the bug. I changed all the words "discount" into "Discount" in cmake/FindDiscount.cmake and it works. The problem seems to be that, when running find_package(Discount), cmake will search for the macro "Discount_FOUND" in the cache. But in FindDiscount.cmake, the macro it defines is "discount_FOUND".

BTW, I think the package properties are useful, so I just added them.

sirgienko added a comment.EditedAug 14 2018, 4:24 PM

@kqwyf, after previous two diff changes, Markdown entries starts don't work for me (don't evaluate), is they works on your machine?

I think problem, that
#cmakedefine discount_FOUND 1 don't work now, because we start use Discount_FOUND.

kqwyf updated this revision to Diff 39739.Aug 14 2018, 4:31 PM
  • update macros about Discount
kqwyf added a comment.EditedAug 14 2018, 4:33 PM

@kqwyf, after previous two diff changes, Markdown entries starts don't work for me (don't evaluate), is they works on your machine?

I think problem, that
#cmakedefine discount_FOUND 1 don't work now, because we start use Discount_FOUND.

I'm so sorry that I didn't build it again after I have done some changes. This time it is built and tested.

Thank you very much. :)

@kqwyf, after previous two diff changes, Markdown entries starts don't work for me (don't evaluate), is they works on your machine?

I think problem, that
#cmakedefine discount_FOUND 1 don't work now, because we start use Discount_FOUND.

I'm so sorry that I didn't build it again after I have done some changes. This time it is built and tested.

Thank you very much. :)

Don't worry, sometimes it happens, I think you have done good job for this task :)

@kqwyf maybe save evaluation result like LaTeX entry does it? If we run entry and save worksheet, I think, after open it, we want to see already 'rendered' entry (and if we don't run markdown entry, we want to see markdown code).
It's possible, if then entry already runned, we could save html and if entry don't runned, we save plain text (we do something similar for LaTeX entry: save latex code or rendered eps file)
Also it's will allow to share worksheet with evaluated markdown entries to another user, who even don't have Discount library.

pino added a comment.Aug 14 2018, 4:52 PM

Nitpick: please provide a consistent indentation in the new files -- they seem to mix spaces and tabs.
Also, was this tested when cantor is built without the Discount library? How does it behave when trying to add/edit markdown entries, and trying to load worksheets with such entries?

src/CMakeLists.txt
104

Extra whitespace change.

108

FindDiscount.cmake creates a Discount::Lib imported library that you can use, and it's even better to do so: it makes sure the target has both the right include directories and the right libraries needed.

src/cantor_part.rc
2

Since new actions are added here, you must bump the version number of the ui .rc file, otherwise users with a customized menubar/toolbar will not see any change.

src/markdownentry.cpp
32

This can be removed, once the extra debug is removed too.

57

Extra debug, no need for it.

64

Ditto.

79

This is leaked in all the cases (i.e. when mkd_compile succeedes or not). You need to call mkd_cleanup().

kqwyf added a comment.EditedAug 14 2018, 5:26 PM
In D14738#309034, @pino wrote:

Nitpick: please provide a consistent indentation in the new files -- they seem to mix spaces and tabs.
Also, was this tested when cantor is built without the Discount library? How does it behave when trying to add/edit markdown entries, and trying to load worksheets with such entries?

When it is build without the Discount library, the markdown entries behave just like text entries when trying to add/edit/load them.

Another bug was found: when re-editing a markdown entry in a Cantor without Discount library, the latex code will disappear. This is because some variables (e.g. html) and codes only for markdown renderring are still in use. I will include all the overridden functions (except toXml() and setContent()) in #ifdef. This way it will work just as the TextEntry, and the content can be interpreted as markdown code in another Cantor with Discount.

Sorry but I'm not sure about the rule to bump the ui version number. What number should it become? Maybe 2.1?

pino added a comment.Aug 14 2018, 5:42 PM

Sorry but I'm not sure about the rule to bump the ui version number. What number should it become? Maybe 2.1?

Only integers are allowed, so 3.

This revision was not accepted when it landed; it landed in state Needs Review.Aug 15 2018, 10:26 AM
This revision was automatically updated to reflect the committed changes.
pino added a comment.Aug 15 2018, 10:29 AM

@filipesaraiva you pushed a patch

  • without all the changes required (see my note about bumping the UI version in cantor_part.rc)
  • with known memory leaks
  • without the proper authorship

good job ...

filipesaraiva reopened this revision.EditedAug 15 2018, 10:33 AM

Oh no, sorry. I thought the authorship was maintained... when I answer:

This branch has revision 'D14738: Add the markdown entry' but you are not

the author. Land this revision by kqwyf? [y/N] y

I thought the authorship would be changed to @kqwyf.

Can I change the authorship of the commit and push now? Do you know @pino?

Argh... really, sorry for this.

pino added a comment.Aug 15 2018, 10:39 AM

Can I change the authorship of the commit and push now? Do you know @pino?

You definitely cannot change already pushed commits...

  1. revert the commit, because of wrong authorship, and unfixed issues still there
  2. wait for @kqwyf to fix the issues
  3. once it is done, ask him/her for name and email (since silly phabricator cannot provide this info for commits sent with arc)
  4. take the patch, amend it fixing the authorship, and push
In D14738#309525, @pino wrote:

Can I change the authorship of the commit and push now? Do you know @pino?

You definitely cannot change already pushed commits...

  1. revert the commit, because of wrong authorship, and unfixed issues still there
  2. wait for @kqwyf to fix the issues
  3. once it is done, ask him/her for name and email (since silly phabricator cannot provide this info for commits sent with arc)
  4. take the patch, amend it fixing the authorship, and push

Ok, thanks for answer @pino. The e-mails of this review was stored in a different folder of my mailbox, because it I thought nobody was asking for more changes.

Sorry again, and please continue the review guys, I will fix it when everyone has approved.

Sorry again, and please continue the review guys, I will fix it when everyone has approved.

Did it means, that you push revert commit after accepting this revision? And if it true, why don't do it now, for avoiding commits between the commit and revert commit?

In D14738#309525, @pino wrote:

Can I change the authorship of the commit and push now? Do you know @pino?

You definitely cannot change already pushed commits...

  1. revert the commit, because of wrong authorship, and unfixed issues still there

Reverted by R55:cfc9324d30dd7b2172c33c7709202a663191a42c

kqwyf added a comment.Aug 15 2018, 2:55 PM
This comment was removed by kqwyf.
kqwyf updated this revision to Diff 39797.Aug 15 2018, 3:47 PM
  • change all the indentation tabs into spaces
  • remove extra changes
  • update CMakeLists.txt
  • bump the ui version in cantor_part.rc
  • fix the memory leak when converting markdown to HTML
  • feature: save evaluation results for markdown entries
kqwyf added a comment.Aug 15 2018, 3:50 PM
  • change all the indentation tabs into spaces
  • remove extra changes
  • update CMakeLists.txt
  • bump the ui version in cantor_part.rc
  • fix the memory leak when converting markdown to HTML
  • feature: save evaluation results for markdown entries

In this patch, a markdown entry without Discount support behaves just like a text entry, except that the latex formulas can be re-edited. I think it's a useful feature.

In this patch, a markdown entry without Discount support behaves just like a text entry, except that the latex formulas can be re-edited. I think it's a useful feature.

Nitpick: Maybe, then user without Discount edit rendered markdown entry from loaded worksheet, we should drop rendered result? So, users don't forget reevaluate this edited entry later.

@kqwyf, Could you add feature, that if user's cursor comes from cell below, cursor appears not in begin of text, but in end?
Another type of entries have this functional, including text entry, so, I think, it's not very difficult to add this functional for MarkdownEntry.

Example worksheet (move the cursor between cells to see how it works):

kqwyf updated this revision to Diff 39821.Aug 16 2018, 2:59 AM
  • fix: focus appears at wrong position in markdown entries
kqwyf updated this revision to Diff 39824.Aug 16 2018, 3:22 AM
  • update the implementation of the last fix
kqwyf added a comment.Aug 16 2018, 6:32 AM

In this patch, a markdown entry without Discount support behaves just like a text entry, except that the latex formulas can be re-edited. I think it's a useful feature.

Nitpick: Maybe, then user without Discount edit rendered markdown entry from loaded worksheet, we should drop rendered result? So, users don't forget reevaluate this edited entry later.

It sounds disturbing... Maybe I should remove this feature when Discount unavailable?

filipesaraiva requested changes to this revision.Aug 16 2018, 8:59 AM

Nitpick: Maybe, then user without Discount edit rendered markdown entry from loaded worksheet, we should drop rendered result? So, users don't forget reevaluate this edited entry later.

This idea is consistent to the current behaviour of LaTeX entries. If someone edit a LaTeX entry (double click on the rendered EPS), Cantor lost that previous rendered result and the user must to process the command again.

For it, markdown feature must to drop the automatic change to edit markdown entry when the user select it. Could @kqwyf implement this using the same behaviour of LaTeX entry?

This revision now requires changes to proceed.Aug 16 2018, 8:59 AM
kqwyf added a comment.EditedAug 16 2018, 9:35 AM

Could @kqwyf implement this using the same behaviour of LaTeX entry?

So the behavior should be:

  • If the user focus on a markdown entry, the rendered content will remains there.
  • If the user double click on it, the plain markdown code will replace the rendered content in the entry, and the user can edit and re-evaluate it.

Is that right?

Besides, to implement the feature above, making a rendered markdown entry uneditable is helpful. Is that okay?

So the behavior should be:

  • If the user focus on a markdown entry, the rendered content will remains there.
  • If the user double click on it, the plain markdown code will replace the rendered content in the entry, and the user can edit and re-evaluate it.

    Is that right?

Right!

Besides, to implement the feature above, making a rendered markdown entry uneditable is helpful. Is that okay?

If "rendered markdown entry uneditable" is like LaTeX entry behaviour, that is ok. :)

kqwyf added a comment.Aug 16 2018, 5:03 PM

Here comes a problem...

The reason I'd like to make rendered markdown entries uneditable is that, if the user edited a rendered markdown entry, then we could never convert the content (in html) back to markdown.
To imitate the behaviors of latex entries better, I thought it's a good idea that the cursor can move on the markdown entries while they can't be edited.

But in worksheettextitem.cpp, line 433-437, it wrote:

void WorksheetTextItem::keyPressEvent(QKeyEvent *event)
{
    if (!isEditable())
        return;
    // other code
}

which means that the cursor can't move on an uneditable entry.

A compromise solution is to make MarkdownEntry::wantFocus() return false when the entry is uneditable. And this will cause the cursor to skip the rendered markdown entries. Is that okay?

BTW, when loading a rendered markdown entry, the markdown content will be rendered, but the latex formulas will remain in code form (just like the text entries). Is that okay? Or should they be rendered (by saving all the eps files in the archive), too?

Here comes a problem...

The reason I'd like to make rendered markdown entries uneditable is that, if the user edited a rendered markdown entry, then we could never convert the content (in html) back to markdown.
To imitate the behaviors of latex entries better, I thought it's a good idea that the cursor can move on the markdown entries while they can't be edited.

In this moment, if you haven't libspectre (or latex), you can't exec latex entry (well, you can, but nothing happens) and another actions allowed. If you haven't libspectre and load worksheet with
latex formulas, we can see rendrered images (okay, in this moment, we can't see, but I working on this problem and this will be fix soon on master), but I could edit unrendered latex entries and convert renderered to unrendered entries by double click.

But in worksheettextitem.cpp, line 433-437, it wrote:

void WorksheetTextItem::keyPressEvent(QKeyEvent *event)
{
    if (!isEditable())
        return;
    // other code
}

which means that the cursor can't move on an uneditable entry.

As I know, LaTeX entries always set Qt::TextEditorInteraction (see LatexEntry constructor), so isEditable() return true, so I think, this code don't give us problems.
And as I see, Text entries do the same.

A compromise solution is to make MarkdownEntry::wantFocus() return false when the entry is uneditable. And this will cause the cursor to skip the rendered markdown entries. Is that okay?

LatexEntry and TextEntry don't override wantFacuse function from WorksheetEntry, so I don't think, that we need override the function in MarkdownEntry.

BTW, when loading a rendered markdown entry, the markdown content will be rendered, but the latex formulas will remain in code form (just like the text entries). Is that okay? Or should they be rendered (by saving all the eps files in the archive), too?

Sorry, but I think, I don't understand your correctly. Rendered LatexEntry saves eps file to archive. As I know Discount generate full html output from markdown, so Markdown Entry haven't any eps files, isn't it?

kqwyf added a comment.EditedAug 17 2018, 12:07 AM

Here comes a problem...

The reason I'd like to make rendered markdown entries uneditable is that, if the user edited a rendered markdown entry, then we could never convert the content (in html) back to markdown.
To imitate the behaviors of latex entries better, I thought it's a good idea that the cursor can move on the markdown entries while they can't be edited.

In this moment, if you haven't libspectre (or latex), you can't exec latex entry (well, you can, but nothing happens) and another actions allowed. If you haven't libspectre and load worksheet with
latex formulas, we can see rendrered images (okay, in this moment, we can't see, but I working on this problem and this will be fix soon on master), but I could edit unrendered latex entries and convert renderered to unrendered entries by double click.

Actually, for now most behaviors of a markdown entry are similar to those of a latex entry. The only difference is that when rendered, you can move the cursor in a latex entry and edit the content (by treating the rendered content as a picture) of a latex entry, but you can't move the cursor nor edit the rendered content (except by double clicking) in a markdown entry.
The reason that the rendered markdown entry shouldn't be edited directly is that it can't be treated as a whole. If my explanation is poor, here's an example (supposing that the markdown entry is always editable and the user has Discount library) :

  1. The user inserted a markdown entry, inputed ## Markdown and evaluated it. So the content was <h2>Markdown</h2> in html.
  2. The user single clicked on it (the entry remains rendered) and deleted some characters, for example, down. The content became <h2>Mark</h2> in html.
  3. The user double clicked on it. In this moment, the user might expect that the content becomes ## Mark, but actually we can't do it because we can't convert html to markdown. What we have is just the original code, ## Markdown.

Due to the reason above, I set Qt::TextBrowserInteraction when the markdown entry is rendered.

As I know, LaTeX entries always set Qt::TextEditorInteraction (see LatexEntry constructor), so isEditable() return true, so I think, this code don't give us problems.
And as I see, Text entries do the same.

Due to the reason above, markdown entries have to be different from others...

LatexEntry and TextEntry don't override wantFacuse function from WorksheetEntry, so I don't think, that we need override the function in MarkdownEntry.

For the problem above, one solution is to override this function to enable the cursor to move on the whole worksheet. (or the cursor will be stopped by a markdown entry)

I just came up with another solution which enable the cursor to move onto the markdown entries (and I tested it). We can catch the KeyPressEvent of m_textItem in the event filter (see src/markdownentry.cpp, line 139) and let the cursor move correctly. But as I think, this solution will make the event processing logic messy and confusing.

What should I do?

Sorry, but I think, I don't understand your correctly. Rendered LatexEntry saves eps file to archive. As I know Discount generate full html output from markdown, so Markdown Entry haven't any eps files, isn't it?

Sorry for my poor explanation.

I'm implementing MarkdownEntry with both markdown and latex support. I tested Discount with flag MKD_LATEX using a string like "$$\sum$$" and it doesn't work. I'm not sure what's wrong, but I chose another solution to get consistent behaviors. The solution is simple: firstly convert the markdown code (with latex formulas in $$) to html by Discount, and then render the latex code by TextEntry::evaluate(), which generates several temporary eps files.

kqwyf updated this revision to Diff 39918.Aug 17 2018, 12:22 PM
  • feature(incomplete): user can edit a markdown entry by double clicking

This version is incomplete, just for discussion.

kqwyf marked 3 inline comments as done.Aug 17 2018, 12:32 PM

The two solutions are in the code. You can uncomment one of them to apply it.

src/markdownentry.cpp
183

Above is the second solution: catch the KeyPress event and move the cursor.

196

Above is the first solution: let wantFocus() return false when it's uneditable.

Here comes a problem...

The reason I'd like to make rendered markdown entries uneditable is that, if the user edited a rendered markdown entry, then we could never convert the content (in html) back to markdown.
To imitate the behaviors of latex entries better, I thought it's a good idea that the cursor can move on the markdown entries while they can't be edited.

In this moment, if you haven't libspectre (or latex), you can't exec latex entry (well, you can, but nothing happens) and another actions allowed. If you haven't libspectre and load worksheet with
latex formulas, we can see rendrered images (okay, in this moment, we can't see, but I working on this problem and this will be fix soon on master), but I could edit unrendered latex entries and convert renderered to unrendered entries by double click.

Actually, for now most behaviors of a markdown entry are similar to those of a latex entry. The only difference is that when rendered, you can move the cursor in a latex entry and edit the content (by treating the rendered content as a picture) of a latex entry, but you can't move the cursor nor edit the rendered content (except by double clicking) in a markdown entry.
The reason that the rendered markdown entry shouldn't be edited directly is that it can't be treated as a whole. If my explanation is poor, here's an example (supposing that the markdown entry is always editable and the user has Discount library) :

  1. The user inserted a markdown entry, inputed ## Markdown and evaluated it. So the content was <h2>Markdown</h2> in html.
  2. The user single clicked on it (the entry remains rendered) and deleted some characters, for example, down. The content became <h2>Mark</h2> in html.
  3. The user double clicked on it. In this moment, the user might expect that the content becomes ## Mark, but actually we can't do it because we can't convert html to markdown. What we have is just the original code, ## Markdown.

    Due to the reason above, I set Qt::TextBrowserInteraction when the markdown entry is rendered.

As I know, LaTeX entries always set Qt::TextEditorInteraction (see LatexEntry constructor), so isEditable() return true, so I think, this code don't give us problems.
And as I see, Text entries do the same.

Due to the reason above, markdown entries have to be different from others...

LatexEntry and TextEntry don't override wantFacuse function from WorksheetEntry, so I don't think, that we need override the function in MarkdownEntry.

For the problem above, one solution is to override this function to enable the cursor to move on the whole worksheet. (or the cursor will be stopped by a markdown entry)

I just came up with another solution which enable the cursor to move onto the markdown entries (and I tested it). We can catch the KeyPressEvent of m_textItem in the event filter (see src/markdownentry.cpp, line 139) and let the cursor move correctly. But as I think, this solution will make the event processing logic messy and confusing.

What should I do?

Okey, I got it. Well, LatexEntry don't protect rendered content from editing (and it Cantor's architectural miscalculation), so maybe MarkdownEntry also don't need it? In this revision, I mean.
The solution of this problem (more integrated and consistent with another code) could be added later and applied for markdown and latex entries (so, I think, @kqwyf, you don't need solve this problem)

kqwyf added a comment.Aug 17 2018, 4:05 PM

Okey, I got it. Well, LatexEntry don't protect rendered content from editing (and it Cantor's architectural miscalculation), so maybe MarkdownEntry also don't need it? In this revision, I mean.
The solution of this problem (more integrated and consistent with another code) could be added later and applied for markdown and latex entries (so, I think, @kqwyf, you don't need solve this problem)

I got it. Thanks. I'll finish the code soon.

sirgienko added a comment.EditedAug 17 2018, 4:18 PM

Sorry, but I think, I don't understand your correctly. Rendered LatexEntry saves eps file to archive. As I know Discount generate full html output from markdown, so Markdown Entry haven't any eps files, isn't it?

Sorry for my poor explanation.

I'm implementing MarkdownEntry with both markdown and latex support. I tested Discount with flag MKD_LATEX using a string like "$$\sum$$" and it doesn't work. I'm not sure what's wrong, but I chose another solution to get consistent behaviors. The solution is simple: firstly convert the markdown code (with latex formulas in $$) to html by Discount, and then render the latex code by TextEntry::evaluate(), which generates several temporary eps files.

Well, I think, we should try to use Discount Latex support, it's more simple than using our code for latex (because, if Discount really used mathjax, we will got uniform html output even we use latex in our Markdown Entry. Without handling with eps files, saving or loading them, etc.)
And if we don't achive success with Discount latex, we still have latex in text result, LatexEntry, so I don't think, that it is big problem using Markdown without Latex. And, after refactoring code for latex rendering in future, it's will be easy to use latex rendering code from TextEntry (but in this moment it isn't so easy).

@pino, do you know somethinkg about embedding Latex into Discount?

kqwyf updated this revision to Diff 40020.Aug 20 2018, 2:06 AM
  • make MarkdownEntry editable
  • remove latex support from MarkdownEntry
kqwyf added a comment.Aug 20 2018, 2:37 AM

There may not be an elegant solution (without refactoring) to make the rich text action list support the markdown entry... One solution is to disable the action list for markdown entry. Is that okay?

BTW, if we disabled the action list, the markdown entry would be less like the text entry. (Actually the first difference is that it doesn't support latex rendered by epsRenderer) Maybe it should extend not TextEntry but WorksheetEntry?

Well, I really don't like the behaviour of Cantor processing LaTeX and text together in a same text entry. For me it is a very confuse behaviour, mainly for now when we have Markdown options for rendering too.

I would prefer a proper LaTeX entry for LaTeX stuffs and text/markdown entries for their respective stuffs. But it was how the original team behind Cantor developed it.

There may not be an elegant solution (without refactoring) to make the rich text action list support the markdown entry... One solution is to disable the action list for markdown entry. Is that okay?

Do you talk about acceptRichText function?

BTW, if we disabled the action list, the markdown entry would be less like the text entry. (Actually the first difference is that it doesn't support latex rendered by epsRenderer) Maybe it should extend not TextEntry but WorksheetEntry?

Well, It's not a problem.

kqwyf added a comment.EditedAug 21 2018, 1:18 PM

Do you talk about acceptRichText function?

Yes. My opinion is to override it and let it return false.
Take Bold for example. When the user selects a string (e.g. text) and clicks the button Bold, he/she may expect that the string becomes **text**, which is difficult to implement in the current architecture of Cantor, I think.

Do you talk about acceptRichText function?

Yes. My opinion is to override it and let it return false.
Take Bold for example. When the user selects a string (e.g. text) and clicks the button Bold, he/she may expect that the string becomes **text**, which is difficult to implement in the current architecture of Cantor, I think.

I see. So, I think, MarkdownEntry could inherit from WorksheetEntry and return false for the rich text function, as you have suggested.

kqwyf updated this revision to Diff 40231.Aug 22 2018, 2:43 PM
  • disable rich text for MarkdownEntry
  • make MarkdownEntry inherit from WorksheetEntry
kqwyf updated this revision to Diff 40234.Aug 22 2018, 3:11 PM
  • restore the access modifiers of TextEntry
sirgienko added inline comments.Aug 22 2018, 3:56 PM
src/markdownentry.h
35

This type already exist. (placeholderentry.h:32)
So, +7.

kqwyf updated this revision to Diff 40267.Aug 23 2018, 2:16 AM
  • update the type value of MarkdownEntry
sirgienko accepted this revision as: sirgienko.Aug 23 2018, 10:09 AM

Good job!
So, we wait, that others will decide.

filipesaraiva accepted this revision.Aug 23 2018, 12:09 PM

Thank you again @kqwyf, the markdown support is a very wished feature and you did a good job!

I will merge into master branch soon if there is no more requests for changes.

Welcome to Cantor development, there are a lot of features to be implemented and bugs to be fixed. :) See some of them in Cantor's Workboard.

This revision was not accepted when it landed; it landed in state Needs Review.Aug 24 2018, 1:09 AM
Closed by commit R55:ea6b09e9e5c9: Add the markdown entry (authored by kqwyf, committed by filipesaraiva). · Explain Why
This revision was automatically updated to reflect the committed changes.

@kqwyf, Congratulations!

Hi, @kqwyf, as I see, your GitHub account name is kqwyf, right?
As I see, Github don't connect your commit with your github account, so author this patch on Github is 1160300905.
Should we try to fix this injustice :) and connect your github account with commit author via github support?

Hi, @kqwyf, as I see, your GitHub account name is kqwyf, right?
As I see, Github don't connect your commit with your github account, so author this patch on Github is 1160300905.
Should we try to fix this injustice :) and connect your github account with commit author via github support?

@kqwyf, just star the Cantor's mirror repository at Github. If your e-mail used in the commit is associated to you Github account, it is enough. :)

Hi, @kqwyf, as I see, your GitHub account name is kqwyf, right?
As I see, Github don't connect your commit with your github account, so author this patch on Github is 1160300905.
Should we try to fix this injustice :) and connect your github account with commit author via github support?

I'm sorry to reply so late!!

Thank you for caring it for me. Actually the Github accounts "kqwyf" and "1160300905" are both mine. The latter is signed up for some reason.

I modified the email address associated to these accounts and it works.

Thank you again for your help! :) @sirgienko @filipesaraiva