Editable --report path - Githubissues

Morgy93 commented 8 months ago

Problem Description

I am using docker and want to run phpcs inside the container. Currently it generates a command like:

phpcs -q --report=/Users/thomas/.vscode-insiders/extensions/obliviousharmony.vscode-php-codesniffer-2.2.0/assets/phpcs-integration/VSCode.php --runtime-set ignore_warnings_on_exit 1 --runtime-set ignore_errors_on_exit 1 --standard=phpcs-ruleset.xml

But the --report= path points to a location on the host and not on the container.

Proposed Solution

I'd like to be able to edit the path used for --report somehow.

Maybe some custom path or replacement like path mapping.

ObliviousHarmony commented 8 months ago

I'll think about how best we might approach this. Out of curiosity, since you're using Docker, have you taken a look at VS Code's Dev Containers extension? This is probably a better option. It works by running a VS Code server in the container and using your local IDE to interact with it. You can run any VS Code extension perfectly without having to worry about things like this.

Morgy93 commented 8 months ago

I'll think about how best we might approach this. Out of curiosity, since you're using Docker, have you taken a look at VS Code's Dev Containers extension? This is probably a better option. It works by running a VS Code server in the container and using your local IDE to interact with it. You can run any VS Code extension perfectly without having to worry about things like this.

Yes, but I am using https://github.com/ddev/ddev which is "meant to be" run on the host machine. I cannot share commands and stuff with colleagues that use PhpStorm. Plus, using many projects I find it tedious to configure every Dev Container on its own. (Like extensions and config)

I really like that feature, but currently it does not fit in our workflow. So hoping for another solution here. 😊

ObliviousHarmony commented 8 months ago

I don't think ddev is mutually exclusive with remote containers. When I looked through their repository for an indication of whether or not they had any guidance, a maintainer mentioned the use of this extension.

There's a guide on attaching to Docker containers and so you should be able to use your ddev container remotely. As for the tedium of configuration, Microsoft's documentation indicates there's some great support for using the same extensions and configuration across containers. Is this something you've tried out before?

Morgy93 commented 8 months ago

I don't think ddev is mutually exclusive with remote containers. When I looked through their repository for an indication of whether or not they had any guidance, a maintainer mentioned the use of this extension.

There's a guide on attaching to Docker containers and so you should be able to use your ddev container remotely. As for the tedium of configuration, Microsoft's documentation indicates there's some great support for using the same extensions and configuration across containers. Is this something you've tried out before?

Like I said, it does not fit into our workflow. Not everyone is using VSCode and for example we rely heavily on the DDEV commands feature where some are run on the host exclusively. I don't say that it is not possible, just that it would cause more trouble to use it, currently. But thanks for the heads up, I'll put it on our DevOps list.

ObliviousHarmony commented 8 months ago

Sure, sure @Morgy93.

On the subject of this particular request, to be clear, would I be right in assuming you are mounting /Users/thomas/.vscode-insiders/extensions/obliviousharmony.vscode-php-codesniffer-2.2.0/assets/phpcs-integration/ somewhere in the Docker container and wanting to reference the path that it's mapped to? This is a trivial request, however, I've strived to keep the configuration of this extension as simple as possible while also offering the flexibility needed to support most users. I'll think about how I can add a setting like this while still achieving that goal.

Morgy93 commented 8 months ago

Yes, for me there is some global config at /mnt/ddev_config/ inside the container and I'd probably just symlink it on the host.

So a custom path would work or a path mapping, which just replaces things. I've seen many implementation of sorts and there's not the single best option.

Just an example: Host: /Users/thomas/.vscode-insiders/extensions/obliviousharmony.vscode-php-codesniffer-2.2.0/assets/phpcs-integration/ Container: /mnt/ddev_config/.vscode-insiders/extensions/obliviousharmony.vscode-php-codesniffer-2.2.0/assets/phpcs-integration/

pathMapping: {
    "/Users/thomas/": "/mnt/ddev_config/"
}

ObliviousHarmony commented 7 months ago

I've thought about this a few times @Morgy93 and my main hesitation stems from a desire to avoid configuration bloat. There are a lot of extensions with way too many options that are meant to serve every imaginable use-case. I feel like a whole setting dedicated to changing a path that won't be used by 99.9% of users is a great example of that problem. I've thought through a couple solutions so far:

Environment Variable: My first idea was to introduce an environment variable that you could set to change the directory. This seems pretty well-aligned with how specific this use-case is in the first place but I'm not sure if there's any precedent for using an environment variable in this way.
Special Object Config: Another route could be an object for "special configuration" options like this. Options that have a pretty narrow use-case. These could be documented here and added to this configuration option by anyone who needs to use them.

The best option seems to be the second. It's more accessible than an environment variable and it still creates a single (empty) setting that will only be used by those that have one of these very narrow use-cases.

Morgy93 commented 7 months ago

how specific this use-case is in the first place

I am not sure about that. I know many agencies that work this way. BUT they're using PHPStorm, so no issue there. (Maybe look how it is implemented there?) And most of them lack usage of phpcs altogether. 😅 But the pure basics of using something like macOS, working on the host, and having the project in a docker container with for example DDEV or Warden is widely spread in my "e-commerce working area".

desire to avoid configuration bloat

That's honorable, thank you. But I would still like to be able to use the extension somehow. 😁

The best option seems to be the second.

Fine by me. 😊

ObliviousHarmony commented 7 months ago

I am not sure about that. I know many agencies that work this way. BUT they're using PHPStorm, so no issue there. (Maybe look how it is implemented there?)

One thing to keep in mind is that PHPStorm includes the most comprehensive PHP language features on the market. It is able to use the standard JSON report to generate fixed file content and then apply those fixes to your open document based on your quick fix selection. This has the benefit of not needing to add anything and being able to use PHPCS out of the box.

But the pure basics of using something like macOS, working on the host, and having the project in a docker container with for example DDEV or Warden is widely spread in my "e-commerce working area".

Technically, if you used the VS Code remote extension or mounted these files at the same path in the container it would work without any editing 😅 The problem here is that we need these report files in order to provide the comprehensive feature set that we currently do. You're not wrong though that it's hard to set this extension up for a containerized environment where the interpreter is run separately from VS Code.

ObliviousHarmony commented 7 months ago

I have some ideas @Morgy93.

Ultimately the problem is that we don't know the path to the files since they need to be mapped. Having to map the directory at all is a pain that makes it hard to use the extension out of the box. The latest version lets you manually set a path so that you can set all of this up manually but it's really a hassle.

What if I distributed the report files via a Composer package? You would install it alongside squizlabs/php_codesniffer and then the files could be autoloaded instead of requiring a path reference. This would actually be a pretty easy change to make and would let us support this use-case both as a global install and a project-specific install. We could then use the current files distributed with the extension as a fallback for every other use-case currently supported.

Morgy93 commented 7 months ago

This has the benefit of not needing to add anything and being able to use PHPCS out of the box.

It does (in the case of our environments). Because nothing is installed on the host machine and php/phpcs needs to be run inside the container. There is a big setup section for PhpStorm using DDEV for example: https://ddev.readthedocs.io/en/latest/users/install/phpstorm/#manual-setup (And they use path mappings as well 😅)

mounted these files at the same path in the container it would work without any editing

I did not think about that before. Will try, maybe that's a quick solution that every colleague is able to use. 🤔

The latest version lets you manually set a path so that you can set all of this up manually but it's really a hassle.

I'll try this as well, along with mounting at the default "expected" location.

What if I distributed the report files via a Composer package? You would install it alongside squizlabs/php_codesniffer and then the files could be autoloaded instead of requiring a path reference. This would actually be a pretty easy change to make and would let us support this use-case both as a global install and a project-specific install. We could then use the current files distributed with the extension as a fallback for every other use-case currently supported.

Sounds like a plan, let's try. 😁

In addition: https://marketplace.visualstudio.com/items?itemName=Trunk.io this extension does some interesting stuff. It downloads needed things to the opened repo at .trunk/ But since this would add changes to git, they also add an entry to .git/info/exclude:

# Trunk is in single player mode. See https://docs.trunk.io
/.trunk

It works beautifully and maybe that's something that could be considered here as well. I think you can use the dedicated .vscode/ folder for such things.

ObliviousHarmony commented 7 months ago

I did not think about that before. Will try, maybe that's a quick solution that every colleague is able to use.

It's always the easy solutions we overlook :smile:

It works beautifully and maybe that's something that could be considered here as well. I think you can use the dedicated .vscode/ folder for such things.

Actually, this approach was my first thought :smile: It seemed too heavy-handed and I wasn't necessarily a fan of how it played with a global phpcs install. If you had a multi-project environment with such a configuration then you'd end up with a bunch of copies of the integration files. After diving into this Composer-based approach, however, I think I'm going to go back to copying the files.

Thanks for the discussion @Morgy93, this has helped me get closer to what I think will be a good solution.

ObliviousHarmony commented 7 months ago

Alright @Morgy93,

I've just released version 3.0.1 of the extension. I ended up going with the Composer approach because there are some blocking issues with copying files. Depending on your environment we can't actually know that the files will be accessible if they're in the .vscode directory. In our monorepo, for example, a directory other than the workspace root is used as the mounting point for a container.

You can use it by installing the obliviousharmony/vscode-phpcs-integration package as a dev dependency wherever you're using phpcs from. This could be globally or it could be per-project, I'm not sure which applies to you but you should. Once you've done this you can enable the setting and it will start to use this instead of the provided files.

Hope this all works out and you can use the extension now :smile: I don't think there's much more I could add to this extension.

ObliviousHarmony / vscode-php-codesniffer

Editable --report path #86

Problem Description

Proposed Solution