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

Avoid using whole ctags_target_for_gcc_minus_e for prototype generation #131

Closed
wants to merge 4 commits into from

Conversation

facchinm
Copy link
Member

@facchinm facchinm commented Apr 7, 2016

Ctags is FULL of bugs on c++ parsing (and c++ syntax is a nightmare), sketches are usually way more simple (and easier to debug).
Using a simplified version of the ctags_target_for_gcc_minus_e file which only contains preprocessor directives and the sketch lines helps avoiding most ctags subtle bugs.

if (saveLine || startsWithHashtag(line)) {
minimizedString += line + "\n"
}
if (containsAny(line, tofind)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why no isLineDirective() check here? Sounds like it could somehow trigger unintended matches when the sketch location somehow occurs in the included code somewhere?

@matthijskooijman
Copy link
Collaborator

I finally got around to reviewing this. I think the idea is great and it should indeed solve all kinds of ctags problems, but I have some doubts about the current implementation (as indicated in the previous comments).

@facchinm
Copy link
Member Author

Hi Matthjis,
thanks A LOT for taking the time to review this! The implementation is clearly suboptimal and a lot of corner cases are not correctly handled but I'm happy you appreciate the idea.
"Fake" line directives are saved only to keep things simple (like "save everything which starts with #" ) and saving them bring no side effect.
The functions names need a fix and we really need to write tests for strange path cases, I'll take care of these issues soon 😉

@facchinm facchinm force-pushed the trimmed_ctags_minus branch 4 times, most recently from e1ed222 to db89809 Compare May 3, 2016 16:21
Ctags is FULL of bugs on c++ parsing (and c++ syntax is a nightmare), sketches are usually way more simple (and easier to debug).
Using a simplified version of the ctags_target_for_gcc_minus_e file which only contains preprocessor directives and the sketch lines helps avoiding most ctags subtle bugs.

Signed-off-by: Martino Facchin <m.facchin@arduino.cc>
@facchinm facchinm force-pushed the trimmed_ctags_minus branch 2 times, most recently from 2d7a5be to 8d2eec4 Compare May 3, 2016 16:50
facchinm added 2 commits May 3, 2016 18:56
…duplicates

Signed-off-by: Martino Facchin <m.facchin@arduino.cc>
TestCTagsRunner* use the #line directive generated file as source instead than # $linenumber.
Searching fot the '#' as first char make tests passing

Signed-off-by: Martino Facchin <m.facchin@arduino.cc>
Signed-off-by: Martino Facchin <m.facchin@arduino.cc>
@ArduinoBot
Copy link
Contributor

✅ Build completed.

⬇️ Build URL: http://downloads.arduino.cc/PR/arduino-builder/arduino-builder-131.zip

ℹ️ To test this build:

  1. Replace arduino-builder binary (you can find it where you installed the IDE) with the provided one

@matthijskooijman
Copy link
Collaborator

I separated the fix and testcase into #154 (I merged them into a single commit) and added a bit more verbose commit message.

@matthijskooijman
Copy link
Collaborator

I've improved (pretty much redid) the main subject of this PR in #156, so I'm closing this one in favor of that one.

@cmaglie cmaglie deleted the trimmed_ctags_minus branch January 11, 2018 10:46
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.

3 participants