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

Use vscode upcoming multiline messages #6

Merged
merged 4 commits into from
Sep 20, 2018

Conversation

doudou
Copy link
Member

@doudou doudou commented Jan 3, 2018

Built on top of #5.

This should be left here until the relevant changes in VSCode are itself merged.

Depends on:

@doudou doudou added the wip label Jan 3, 2018
@doudou doudou force-pushed the use_vscode_upcoming_multiline_messages branch 5 times, most recently from 26f732b to 79f4133 Compare January 4, 2018 19:29
@g-arjones g-arjones force-pushed the use_vscode_upcoming_multiline_messages branch from 79f4133 to 87aaf31 Compare March 8, 2018 13:34
@g-arjones g-arjones removed the wip label Mar 8, 2018
@g-arjones
Copy link
Contributor

Multiline problem matchers made it into vscode 1.21. I have rebased this and set problematic files' path as being relative. Although, it seems that it only works when the package is built for the first time.. Could you check that?

@coveralls
Copy link

coveralls commented Mar 8, 2018

Pull Request Test Coverage Report for Build 60

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 79.557%

Totals Coverage Status
Change from base Build 58: 0.0%
Covered Lines: 953
Relevant Lines: 1189

💛 - Coveralls

@g-arjones g-arjones force-pushed the use_vscode_upcoming_multiline_messages branch from 839fbab to 87aaf31 Compare March 8, 2018 15:01
@doudou
Copy link
Member Author

doudou commented Mar 9, 2018

Well ... making paths relative should either not work or be brittle with autoproj, since the autoproj cwd can virtually be anything (and is definitely not guaranteed to be the packages' path :()

We could try the solution suggested here: https://cmake.org/pipermail/cmake/2014-March/057164.html

@doudou
Copy link
Member Author

doudou commented Mar 9, 2018

Note that I don't understand how this would even work with "pure cmake" when using separate build dirs. AFAIK, one has to run within the cmake build dir, so the relative path would not resolve properly either.

@doudou
Copy link
Member Author

doudou commented Mar 9, 2018

For what it's worth, I might start working on attaching whole backtraces to problems in VSCode. It bites me at every turn, whatever the language. C++ template instanciation right now is basically impossible to go through in the problem view.

@doudou
Copy link
Member Author

doudou commented Mar 9, 2018

For reference, the problem matchers for C++ inclusion and template instanciation error traces: #52

@g-arjones
Copy link
Contributor

and is definitely not guaranteed to be the packages' path

Good point. In this case I would remove the "field" property from the pattern (or leave at as relative and just accept to have it working only "sometimes" rather than "never").

C++ template instanciation

Those are a major pain.

Could you get the multiline matchers to work consistently? In my tests I only got it to work once per package (which is pretty weird)

@doudou
Copy link
Member Author

doudou commented Mar 9, 2018

Could you get the multiline matchers to work consistently? In my tests I only got it to work once per package (which is pretty weird)

I haven't re-tried, I really need to focus on the project stuff right now (from which a bunch of syskit and roby bugs get triggered and fixed as you have seen)

@g-arjones
Copy link
Contributor

No problem..

@doudou
Copy link
Member Author

doudou commented Sep 17, 2018

Same here ... I close this, we'll reopen when someone starts working on it again.

@doudou doudou closed this Sep 17, 2018
@g-arjones
Copy link
Contributor

I will do it...

@g-arjones g-arjones reopened this Sep 17, 2018
@g-arjones g-arjones force-pushed the use_vscode_upcoming_multiline_messages branch from 87aaf31 to 77da94e Compare September 20, 2018 02:27
@g-arjones
Copy link
Contributor

g-arjones commented Sep 20, 2018

There was a bug in a regex. Fixed and seems to work now..

Making the file location relative should work in most cases, AFAICT. Autoproj's cwd is not relevant, cmake always output file locations relative to the CMAKE_BINARY_DIR and vscode expands them to ${workspaceFolder}, regardless of the cwd in both cases. Problem is, for the workspace folder to be correct, the task provider must set the task's scope to the folder's path (using the new constructor).

Since we are not doing this yet, I am leaving the file location as absolute for now...

@g-arjones g-arjones merged commit 6ca29e5 into master Sep 20, 2018
@g-arjones g-arjones deleted the use_vscode_upcoming_multiline_messages branch September 20, 2018 02:32
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

3 participants