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

split c.c into two parts #3332

Merged
merged 18 commits into from Apr 22, 2022
Merged

split c.c into two parts #3332

merged 18 commits into from Apr 22, 2022

Conversation

techee
Copy link
Contributor

@techee techee commented Apr 8, 2022

This is what I proposed in

#3327

with some modifications:

  1. I got a bit lazy and didn't remove any code from c.c which I left more or less as it was before - I could remove the extra languages if desired, I'm just not sure if it's worth the work since this file probably won't be actively maintained.
  2. I also removed Vera from c-based.c (and use the parser from c.c). The language seems to be mostly dead without any proper documentation and this makes c-based.c hard to maintain because nobody knows properly what code is there because of Vera and what it should do. c.c seems to be a good graveyard for Vera to me.

I really just fixed the compilation errors and didn't go through the parser in detail - I'm pretty sure that there's some code left which is only specific to C/C++ (but which isn't explicitly guarded for these languages) so there probably are some things which are unnecessary.

@codecov
Copy link

codecov bot commented Apr 8, 2022

Codecov Report

Merging #3332 (4861e8e) into master (2f69ee9) will decrease coverage by 2.27%.
The diff coverage is 72.45%.

@@            Coverage Diff             @@
##           master    #3332      +/-   ##
==========================================
- Coverage   85.44%   83.16%   -2.28%     
==========================================
  Files         216      218       +2     
  Lines       50319    52235    +1916     
==========================================
+ Hits        42995    43442     +447     
- Misses       7324     8793    +1469     
Impacted Files Coverage Δ
parsers/c-based.c 80.59% <ø> (ø)
parsers/c.c 2.72% <ø> (-76.16%) ⬇️
parsers/vera.c 72.45% <72.45%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2f69ee9...4861e8e. Read the comment docs.

@masatake masatake self-requested a review April 9, 2022 01:32
@masatake
Copy link
Member

masatake commented Apr 9, 2022

I'm negative about removing the Vera parser.

How about doing the following?

  1. copy c-base.c from c.c as you did.
    1.1 remove C/C++ specific code from c-base.c as you did.
    1.2 remove parser definitions other than C/C++ from c.c as you did.
  2. copy vera.c from c-base.c.
    2.1 remove parser definitions other than Vera from vera.c.
  3. remove Vera from c-base.c as you did.
  4. do as you planned in Splitting c.c into two parsers (c/c++ parser & the parser of the rest) #3327.

2. and 2.1. are the extra steps. You don't have to touch vera.c more, of course.

The commits up to f2b6293 look good to me.
Thank you.

@techee
Copy link
Contributor Author

techee commented Apr 10, 2022

@masatake Done.

@masatake
Copy link
Member

Thank you for taking your time.
Could you rebase this pull request on the latest master?
@leleliu008 and I fixed the trouble on CI/CD environment targeting FreeBSD / testing (pull_request) .

It will serve as a compiler of languages other than C/C++ from c.c.
Remove Lang_c, Lang_cpp and additional c/c++ defines, C/C++ entries from
the keyword map (which requires updating indexes when calling
buildKeywordHash()) and isKnrParamList (which is K&R style C function
declaration flag).

Fix the resulting compilation errors and make sure units test pass after
this.
This is the only keyword with 0's for all languages in KeywordTable.
…ation

Remove "task" keyword, all Vera declarations and fix compilation errors.
@techee
Copy link
Contributor Author

techee commented Apr 12, 2022

Could you rebase this pull request on the latest master?

Done.

@masatake
Copy link
Member

Thank you. Please, wait for awhile.

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.

Thank you very much for updating vera.c.

@masatake masatake merged commit 3f95c7d into universal-ctags:master Apr 22, 2022
@masatake
Copy link
Member

Thank you very much.

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.

None yet

2 participants