Skip to content

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