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

Initial version #2

Merged
merged 48 commits into from
Jan 3, 2018
Merged

Initial version #2

merged 48 commits into from
Jan 3, 2018

Conversation

doudou
Copy link
Member

@doudou doudou commented Dec 7, 2017

No description provided.

@doudou
Copy link
Member Author

doudou commented Dec 7, 2017

@g-arjones: some helpers that would probably be helpful for you. Ported from https://github.com/rock-core/atom-build-autoproj.

@g-arjones
Copy link
Contributor

Absolutely! This saves me from a lot of work. How does the extension publishing work? Is it automatically available to the world when we push to master? If that's the case, we should probably have a 'develop' branch where we could review each others work and only merge into master when releasing makes sense.

@doudou
Copy link
Member Author

doudou commented Dec 7, 2017

How does the extension publishing work? Is it automatically available to the world when we push to master?

Nope, there's a manual step involved (need to run vcse publish).

@g-arjones
Copy link
Contributor

Perfect... Thanks 👍

@doudou
Copy link
Member Author

doudou commented Dec 7, 2017

Updated with a bit more tooling, and the implementation of the autoproj build targets. Open any package from a workspace, and you'll get build/update/... targets for the whole workspace, and for the packages that are opened.

This is still crude, but alpha-level functional.

Missing:

  • check that folders are actually within a package (I only check that they are within a workspace) and use the package name in the targets
  • handle errors and warnings

Lessons learned:

  • there is no way to add a folder to the workspace through the API, and therefore some functionalities (like the add/remove folder events) can simply not be tested. Best practice is to move as much code as possible out of the VSCode loop into functions that can be tested separately.
  • tasks are identified through their definition field, which means that two task objects that have the same definition will end up showing only once in the build target list.
  • if you rename or remove a test file, you would need to remove them from the out/ folder as well, or the test runner will continue running them.

@g-arjones
Copy link
Contributor

I was going a slightly different path... For large projects, I am not sure if having one task for each folder in the workspace is a good idea (we end up with a lot of tasks). I think we should have build/update tasks for the currently open folder and a global, which builds/updates the whole workspace.

What do you think?

@doudou
Copy link
Member Author

doudou commented Dec 7, 2017

What do you think?

I think that I'd rather go the simple route (a.k.a. "the one I have already implemented") and refine later.

VSCode does show the most used targets first. Moreover, since we have to add the package's folder to their workspace, I assume that we won't end up with a lot of packages in a single workspace. This, added with the ability to filter the targets in the list, seem to me workable.

I have other ideas if this one ends up being not so great. One route would for instance to have a "${TARGET} Package ..." (e.g. "Update Package ...") that lets the user pick a package in a list and remember the last 3-4 targets. Interestingly, this would allow to use targets on packages that are not in the workspace.

@g-arjones
Copy link
Contributor

I have other ideas if this one ends up being not so great. One route would for instance to have a "${TARGET} Package ..." (e.g. "Update Package ...") that lets the user pick a package in a list

I like this one..

Noticed a weird behavior here... Once I use a build task, it is gone from the task list and I can only run the "Force Build" tasks.

@doudou
Copy link
Member Author

doudou commented Dec 8, 2017

Once I use a build task, it is gone from the task list and I can only run the "Force Build" tasks.

I'll check that one out.

One major negative point for VSCode ... the output parsing is horribly limited. See microsoft/vscode#9635 (and a lot of its friends). Not a complete blocker, we could go the Atom route and create our own problem reporting extension, but worth mentioning.

workspaceFolders in this case is undefined, not an empty array
This led to having tasks appear or disappear at a whim.
@doudou
Copy link
Member Author

doudou commented Dec 8, 2017

Once I use a build task, it is gone from the task list and I can only run the "Force Build" tasks.

Caused by duplicate 'mode' field in 'definitions' ... Just pushed a fix. It's critical to have unique 'definition' objects.

(I moved my 'lessons learned' to the wiki, along with some info about autoproj/rock that might be useful for you).

@g-arjones
Copy link
Contributor

I have noticed the output parsing thing when I was writing problem watchers for unit tests. Besides that, did you have anything else in mind that would require parsing multi line messages?

@g-arjones
Copy link
Contributor

Hmm... I didn't go that far. Seems like the situation is worse than I thought.

@doudou
Copy link
Member Author

doudou commented Dec 8, 2017

Working on VSCode itself is a major pain. Basically, the open-source vscode on github is the "OSS edition", which has none of the default extensions installed (partly because some of these extensions is not open source). So, after building the repo (which is OK), you're left with a bare-bone install - that can't even run extensions - and you have to manually install stuff, without the extension gallery.

@doudou
Copy link
Member Author

doudou commented Dec 8, 2017

(Note: I don't know how working on Atom is ...)

@doudou
Copy link
Member Author

doudou commented Dec 8, 2017

Besides that, did you have anything else in mind that would require parsing multi line messages?

Given how important those are, it would be enough. But anyways,

  • associate multiple file/lines to a single message/error (think backtrace of an exception)
  • syskit loading errors are very often multi-line (with backtrace)
  • The output of the autoproj commands.
    • autoproj build builds multiple packages in parallel, so one has to regroup the messages per-package, and then parse them normally.
    • autoproj update shows a single line with the package and then the error message as multi-line
  • GCC suggestions: related to a certain warning or error. There are usually so many of them that it makes going through the error list so bloody confusing. Grouping those under a single item would really help.
  • GCC template instanciation stack: same thing (you know the 20 lines of "instanciated from ..." before you get to the actual problem ?

Having had a glimpse in the build/linter API of Atom, it raised the bar of "exception parsing" pretty high. Parsing is done in Javascript directly, and they have a hierarchical tree for the problem view. However, VSCode's expressiveness is even lower than the one of vim. That was not a bar very high to pass.

I've added some comments to the VSCode issues related to that ... let's see.

@g-arjones
Copy link
Contributor

associate multiple file/lines to a single message/error (think backtrace of an exception)
syskit loading errors are very often multi-line (with backtrace)

Those would be great to have in the problems tab but given vscode limitation, it will probably be very hard. The ruby extension itself does not parse backtraces as problems.

autoproj build builds multiple packages in parallel, so one has to regroup the messages per-package, and then parse them normally.

With the new multi-root workspace API isn't there a way to associate problems with a folder/project?

GCC's output is often hard enough for a human to parse so I really don't expect much in this regard and really have never seen a solution to this other than the usual "severity, file, line, message" parsing.

NOTE: This guy is able to parse this. Maybe we should have a look and how he is doing it...

@doudou
Copy link
Member Author

doudou commented Dec 8, 2017

GCC's output is often hard enough for a human to parse so I really don't expect much in this regard and really have never seen a solution to this other than the usual "severity, file, line, message" parsing.

Well, actually ... I did some prototyping on Atom to at least group suggestions with the error that is being suggested, it clears things up quite significantly already.

@g-arjones
Copy link
Contributor

Sounds innovative to me.

Never mind my note in the previous comment, Catch allows to customize its output to make it easier to parse, that's probably what the extension is doing.

Another workaround would be to not use vscode's task service and register everything as commands instead, spawn the processes ourselves and parse the output. I have no idea how much work that is in TypeScript though..

@doudou doudou changed the title Autoproj helpers to access information about a given workspace Initial version Jan 3, 2018
@doudou doudou merged commit b6b837f into master Jan 3, 2018
@doudou doudou deleted the autoproj branch January 3, 2018 14:14
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