Skip to content

Conversation

@jvsqzj
Copy link
Contributor

@jvsqzj jvsqzj commented Nov 28, 2024

This PR addresses issue #10 - Support single file linting mode that does not recurse through imported files

Signed-off-by: Juan Vasquez juan.j.vasquez.alfaro@intel.com

@JonatanWaern
Copy link
Contributor

Please give the pull request some sort of descriptive name, even if its a draft not ready for review.

@jvsqzj jvsqzj changed the title WIP Supress triggering device analyses and resolving imported files for single file CLI mode Dec 6, 2024
@jvsqzj jvsqzj force-pushed the issue/10-supress-imports-direct-lint-mode branch 2 times, most recently from f4178e7 to f7a1bdb Compare December 6, 2024 23:00
@jvsqzj jvsqzj changed the title Supress triggering device analyses and resolving imported files for single file CLI mode Suppress triggering device analyses and resolving imported files for single file CLI mode Dec 20, 2024
@me-cr me-cr force-pushed the issue/10-supress-imports-direct-lint-mode branch from e25faec to 68d37f9 Compare December 20, 2024 22:12
@jvsqzj
Copy link
Contributor Author

jvsqzj commented Dec 20, 2024

We have suppressed device analyses and any requests being sent from ActionContext into the main server loop. Only step 3 from issue #10 is still missing. This can be optimized further but the single file mode appears to work fast enough for now. Might be worth checking it with a set of multiple files. No imports are analyzed, but the files are still resolved by the file_management logic so the VFS must be spending some cycles on this that won't be used for anything.

@JonatanWaern after doing some work on this, I believe the direct_only flag should be used exclusively for CLI mode, when DLS is triggered with the DFA CLI tool. I see no use in having direct_only set as enabled for working with VS Code or any other client editor. Let us know if we are ignoring something important about it, but to me we should rename it to cli_mode and keep it only for using DFA.

@jvsqzj jvsqzj force-pushed the issue/10-supress-imports-direct-lint-mode branch 2 times, most recently from f671bd7 to 8488cf6 Compare December 20, 2024 22:49
@jvsqzj
Copy link
Contributor Author

jvsqzj commented Dec 28, 2024

We have suppressed device analyses and any requests being sent from ActionContext into the main server loop. Only step 3 from issue #10 is still missing. This can be optimized further but the single file mode appears to work fast enough for now. Might be worth checking it with a set of multiple files. No imports are analyzed, but the files are still resolved by the file_management logic so the VFS must be spending some cycles on this that won't be used for anything.

@JonatanWaern after doing some work on this, I believe the direct_only flag should be used exclusively for CLI mode, when DLS is triggered with the DFA CLI tool. I see no use in having direct_only set as enabled for working with VS Code or any other client editor. Let us know if we are ignoring something important about it, but to me we should rename it to cli_mode and keep it only for using DFA.

I have renamed this boolean to cli_mode. I've been doing some testing, and now this is set as false on the default config.
It should only be set to true for using with DFA.

So far, both CLI mode and the interactive editor mode with the extension work fine with these changes.

@JonatanWaern JonatanWaern marked this pull request as ready for review January 13, 2025 11:52
Copy link
Contributor

@JonatanWaern JonatanWaern left a comment

Choose a reason for hiding this comment

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

Minor changes needed, seems to work overall though.

Copy link
Contributor

@JonatanWaern JonatanWaern left a comment

Choose a reason for hiding this comment

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

Looks good now I think, just needs a rebase on main before merge.

@jvsqzj jvsqzj force-pushed the issue/10-supress-imports-direct-lint-mode branch from 67aadc2 to 4cbb5e6 Compare January 27, 2025 23:21
@alecalvop alecalvop force-pushed the issue/10-supress-imports-direct-lint-mode branch 2 times, most recently from a562496 to 21a0c12 Compare January 27, 2025 23:33
@jvsqzj
Copy link
Contributor Author

jvsqzj commented Jan 27, 2025

Looks good now I think, just needs a rebase on main before merge.

Done, now rebased and tested locally. The CLI mode works as intended.

@jvsqzj jvsqzj force-pushed the issue/10-supress-imports-direct-lint-mode branch 2 times, most recently from 9b37b6f to 6f082cf Compare January 28, 2025 01:08
@jvsqzj jvsqzj force-pushed the issue/10-supress-imports-direct-lint-mode branch from 6f082cf to 8d4ae01 Compare January 28, 2025 01:20
@jvsqzj jvsqzj requested a review from JonatanWaern January 28, 2025 01:21
@jvsqzj jvsqzj force-pushed the issue/10-supress-imports-direct-lint-mode branch from 8d4ae01 to dba6005 Compare January 28, 2025 01:32
@JonatanWaern JonatanWaern merged commit 9d18715 into intel:main Jan 28, 2025
4 checks passed
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.

4 participants