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

Markdown: add hashtags functionality #3747

Merged
merged 3 commits into from
Jun 25, 2023

Conversation

jiangyinzuo
Copy link
Contributor

Some markdown editors or plugins, such as Obsidian and VSCode markdown-hashtags, support parsing #hashtags and navigation. I have modified the markdown parser to support hashtag syntax parsing.

@masatake
Copy link
Member

masatake commented Jun 22, 2023

Looks nice. If you are good at git, polish the pull request with git rebase -i and git push --force.

e9d8451 comes first. Please, use "main,refactor: " as the prefix of the commit header.

8372df3 should comes second. So you can use functions declared in utf8_str.h in the code supporting the hashtag kind. Please, use "Markdown: " as the prefix of the commit header.

Put the sentences in the description of this pull request, "Some markdown editors or plugins, such as Obsidian and VSCode markdown-hashtags, support parsing #hashtags and navigation. I have modified the markdown parser to support hashtag syntax parsing." to the commit log of the second one. It is helpful to understand the commit.

a58be23 should be squashed into the first one.


@masatake's TODO:

  • parser versioning (a kind is added)

Thank you.

@masatake masatake changed the title add hashtags functionality to markdown parser Markdown: add hashtags functionality Jun 22, 2023
@codecov
Copy link

codecov bot commented Jun 23, 2023

Codecov Report

Patch coverage: 95.38% and no project coverage change.

Comparison is base (47398af) 83.01% compared to head (9271582) 83.01%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3747   +/-   ##
=======================================
  Coverage   83.01%   83.01%           
=======================================
  Files         226      227    +1     
  Lines       55086    55126   +40     
=======================================
+ Hits        45727    45764   +37     
- Misses       9359     9362    +3     
Impacted Files Coverage Δ
parsers/asciidoc.c 95.45% <ø> (-0.36%) ⬇️
parsers/rst.c 92.55% <ø> (-0.43%) ⬇️
main/utf8_str.c 89.65% <89.65%> (ø)
parsers/markdown.c 99.59% <100.00%> (+0.07%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@jiangyinzuo
Copy link
Contributor Author

Thanks for your tips! Now the polished PR is ready.

parsers/markdown.c Show resolved Hide resolved
.gitignore Show resolved Hide resolved
parsers/markdown.c Outdated Show resolved Hide resolved
parsers/markdown.c Outdated Show resolved Hide resolved
@jiangyinzuo jiangyinzuo force-pushed the master branch 2 times, most recently from 70ad291 to 8cb18cb Compare June 23, 2023 09:56
Copy link
Member

@masatake masatake left a comment

Choose a reason for hiding this comment

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

Could you add the original comment of this pull request to the commit log? It helps a person understand this pull request.

Some markdown editors or plugins, such as [Obsidian](https://help.obsidian.md/Editing+and+formatting/Tags) and [VSCode markdown-hashtags](https://github.com/vanadium23/markdown-hashtags), support parsing #hashtags and navigation. I have modified the markdown parser to support hashtag syntax parsing.

Tmain/list-fields.d/stdout-expected.txt Outdated Show resolved Hide resolved
Units/parser-markdown.r/hashtags-utf8.d/args.ctags Outdated Show resolved Hide resolved
.enabled = false,
.name = "hashtag",
.description = "#hashtag",
},
Copy link
Member

Choose a reason for hiding this comment

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

(review comment Group 1)

};

typedef enum {
F_MARKER,
F_HASHTAG,
Copy link
Member

Choose a reason for hiding this comment

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

(review comment Group 1)

parsers/markdown.c Outdated Show resolved Hide resolved
parsers/markdown.c Outdated Show resolved Hide resolved
Units/parser-markdown.r/hashtags-utf8.d/expected.tags Outdated Show resolved Hide resolved
@masatake
Copy link
Member

masatake commented Jun 23, 2023

The first commit and the third commit are ready to merge. Thank you.
If you are comfortable, I will cherry-pick the first and third ones immediately.
In that case, let me know.

@jiangyinzuo
Copy link
Contributor Author

The first commit and the third commit are ready to merge. Thank you. If you are comfortable, I will cherry-pick the first and third ones immediately. In that case, let me know.

It's OK

@masatake
Copy link
Member

This one is ready for merging.
Could you give me a time before merging?

I would like to try solving #3748 before merging this. If I find fixing #3748 may take longer time, I will merge this pull request first.

@jiangyinzuo
Copy link
Contributor Author

jiangyinzuo commented Jun 24, 2023

This one is ready for merging. Could you give me a time before merging?

I would like to try solving #3748 before merging this. If I find fixing #3748 may take longer time, I will merge this pull request first.

It's OK, and it is better to cherry-pick the first and third commits. I plan to open a new PR (or continue in this one) for the qualified hashtag feature.

@jiangyinzuo
Copy link
Contributor Author

I force-pushed again. Now the first commit is duplicated with #3749 and the third commit implements the features for qualified hashtags and subtags

masatake added a commit to masatake/ctags that referenced this pull request Jun 24, 2023
Close universal-ctags#3748.

A test input is derrived from Yinzuo Jiang (@jiangyinzuo)'s
commit in universal-ctags#3747.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
masatake added a commit to masatake/ctags that referenced this pull request Jun 24, 2023
Close universal-ctags#3748.

A test input is derrived from Yinzuo Jiang (@jiangyinzuo)'s
commit in universal-ctags#3747.

Signed-off-by: Masatake YAMATO <yamato@redhat.com>
@masatake
Copy link
Member

Could you rebase this pull request on the latest master branch?
You must drop dc0c700 from this pull request.

@jiangyinzuo
Copy link
Contributor Author

Could you rebase this pull request on the latest master branch? You must drop dc0c700 from this pull request.

Done. Could you please review the second commit again?

@@ -57,6 +60,8 @@ static kindDefinition MarkdownKinds[] = {
{ true, 'T', "l4subsection", "level 4 sections" },
{ true, 'u', "l5subsection", "level 5 sections" },
{ true, 'n', "footnote", "footnotes" },
{ true, 'h', "hashtag", "hashtags"},
{ true, 'H', "subtag", "subtags in hashtags"},
Copy link
Member

Choose a reason for hiding this comment

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

Do you think we should have both hashtag and subtag?
Having only hashtag is not enough?

@masatake
Copy link
Member

We are going the wrong way.

Could you push the original version to that the sub-hashtag was not introduced yet?
I will merge the original version.
We can work on the sub-hashtag after merging the original version.

@masatake
Copy link
Member

BTW, I found you are using your master branch. I strongly recommend using a topic branch.
See https://git-scm.com/book/en/v2/Git-Branching-Branching-Workflows

So you can track the change on the master branch of the upstream and develop new features simultaneously.

Some markdown editors or plugins, such as
[Obsidian](https://help.obsidian.md/Editing+and+formatting/Tags) and
[VSCode markdown-hashtags](https://github.com/vanadium23/markdown-hashtags),
support parsing #hashtags and navigation. I have modified the markdown
parser to support hashtag syntax parsing.
@masatake masatake merged commit e39a22b into universal-ctags:master Jun 25, 2023
38 checks passed
@masatake
Copy link
Member

Thank you!

@masatake
Copy link
Member

I have some advises about tagging #a/b.

See php.c:

static scopeSeparator PhpGenericSeparators [] = {
        { K_NAMESPACE        , NAMESPACE_SEPARATOR },
        { KIND_WILDCARD_INDEX, "::" },
};

You can do the same as the PHP parser does:

static scopeSeparator MarkdownHashtagSeparators [] = {
        { K_HASHTAG        , "/" },  /* If a hashtag is the parent. */
        { KIND_WILDCARD_INDEX, "#" }, /* If something other is the parent. */
};
...
static kindDefinition MarkdownKinds[] = {
        { true, 'c', "chapter",       "chapters"},
        { true, 's', "section",       "sections" },
        { true, 'S', "subsection",    "level 2 sections" },
        { true, 't', "subsubsection", "level 3 sections" },
        { true, 'T', "l4subsection",  "level 4 sections" },
        { true, 'u', "l5subsection",  "level 5 sections" },
        { true, 'n', "footnote",      "footnotes" },
        { true, 'h', "hashtag",       "hashtags"
           ATTACH_SEPARATORS(MarkdownHashtagSeparators)}},

With this change, ctags generate full-qualified tags like #a/b with --extras=+qufalifed if you parser does:

  • making a tag for a with the hashtag kind, and
  • making a tag for b with the hashtag kind and with setting a to b's scope.

When making a tag for a, makeTagEntry or something returns an integer.
We call it a corkIndex for a.
Set the corkIndex for a to extensionFields.scopeIndex of b when making a tag for b.
See makeMarkdownTag.

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.

2 participants