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

Add CLI flag to configure relative vs full path printing for emitters #14985

Open
brupelo opened this issue Dec 15, 2024 · 6 comments
Open

Add CLI flag to configure relative vs full path printing for emitters #14985

brupelo opened this issue Dec 15, 2024 · 6 comments
Labels
needs-info More information is needed from the issue author

Comments

@brupelo
Copy link

brupelo commented Dec 15, 2024

Follow-up from #9417

I’ve added a new flag to the CLI that allows configuring the Printer to print either relative or full paths across the different existing emitters.

The problem with relative paths is that they are not always convenient, especially when linting in an IDE or when dealing with additional cases where valuable information is lost. This is particularly relevant when parsing paths from Ruff's stdout, which can be useful for feeding into LLMs. There are also situations where full paths are necessary to redirect Ruff’s stdout, but the information is lost when using relative paths. Furthermore, we can’t always assume that the working directory of Ruff’s execution is the same as the working directory where the pyproject.toml file resides.

While relative paths are more compact, they introduce a variety of limitations in different use cases. Therefore, it would be beneficial to retain existing functionality while adding the option to configure how paths are printed.

I’ve created a draft PR for you to review so you can see where I am with this implementation.

Draf PR here -> #14987

@MichaReiser
Copy link
Member

MichaReiser commented Dec 15, 2024

Thanks for opening this issue.

Would you mind sharing a few examples for

The problem with relative paths is that they are not always convenient, especially when linting in an IDE or when dealing with additional cases where valuable information is lost.

Most IDEs have good support for relative links and you can click them to jump right to the file. This works for as long as ruff is run from the same directory as the IDE's working directory.

Furthermore, we can’t always assume that the working directory of Ruff’s execution is the same as the working directory where the pyproject.toml file resides.

I don't think Ruff makes this assumption. You can see how ruff always uses paths relative to the current working directory. The location of the pyproject.toml doesn't matter. (I ran the command from the root directory)

~/astral/ruff/target/debug/ruff check --cache-dir ~/astral/test/.ruff_cache ~/astral/test --config ~/astral/test/pyproject.toml --select=ALL
warning: `one-blank-line-before-class` (D203) and `no-blank-line-before-class` (D211) are incompatible. Ignoring `one-blank-line-before-class`.
warning: `multi-line-summary-first-line` (D212) and `multi-line-summary-second-line` (D213) are incompatible. Ignoring `multi-line-summary-second-line`.
warning: Detected debug build without --no-cache.
Users/micha/astral/test/test.py:1:1: INP001 File `Users/micha/astral/test/test.py` is part of an implicit namespace package. Add an `__init__.py`.
Users/micha/astral/test/test.py:1:1: D100 Missing docstring in public module
Users/micha/astral/test/test.py:1:1: I001 [*] Import block is un-sorted or un-formatted

@MichaReiser MichaReiser added cli Related to the command-line interface wish Not on the current roadmap; maybe in the future labels Dec 15, 2024
brupelo pushed a commit to brupelo/ruff that referenced this issue Dec 15, 2024
@brupelo
Copy link
Author

brupelo commented Dec 15, 2024

@MichaReiser Thanks for the questions! I'll come back to this over the next few days with more details so you can better understand the value added. Could you please keep the PR open until this issue is resolved?

The idea is straightforward: I want to extend Ruff's behavior with a new flag that doesn’t break any existing functionality. So, the default behavior and existing formatters would remain as they are, and this would be an additive feature aimed at a niche use case.

It's a bit difficult for me to provide a complete example right now as I need to make some changes to the code, rebuild it, and prepare relevant examples. I can say that adding this flag to the ongoing PR would take me about 10 minutes, but I haven’t reached that part of the codebase yet. I'm not sure how hard it will be to add the Clap flags without breaking anything. As mentioned, I’m still a beginner with Rust, and what I thought would be a 30-minute task has already taken me a few hours, and I've definitely picked up some antipatterns along the way. Please bear with me 😅

That said, it’s been a really cool project to contribute to. One challenge I’m facing is reducing iteration times in Rust, especially when compiling top-level crates.

Also, today I was trying to get Sublime LSP analyzer to work with imports but had no luck. Out of curiosity, what’s the typical IDE and environment used by Ruff contributors for efficient iteration and refactoring?

It would be great to have some tips for new contributors so they can onboard quickly.

Again, thanks for your time. I’m excited about getting this feature into the main branch. I want to use Ruff in place of my old {black, isort, autoflake8, flake8} combo and be able to run and refactor dozens of projects in different repos efficiently, without breaking a sweat. I’m confident that Ruff will make this process even faster.

P.S. To keep it simple, imagine you want to lint several large codebases, aggregate the metrics, and then reformat the output to pass to another tool for specific reasons. Right now, the process is very rigid. I recall some linters offering configurability to adjust how paths are output by the emitters. Would that work better for you instead a simple boolean flag? Essentially, something similar to how Python’s logging formatter gives users the flexibility to customize the output. Again, the default behavior would remain as is, with no breaking changes.

brupelo pushed a commit to brupelo/ruff that referenced this issue Dec 16, 2024
brupelo pushed a commit to brupelo/ruff that referenced this issue Dec 16, 2024
@brupelo
Copy link
Author

brupelo commented Dec 16, 2024

I've cleaned up a bit my feature branch so it now uses env.vars instead of the previous hardcoded solution, here's some fast testing:

========================================
File: a.py
========================================

x()x()

----------------------------------------


========================================
File: Justfile
========================================

    config_file := "d:/pyproject.toml"
    ruff_file := "ruff"

    test-abs:
        @RUFF_SHOW_FULL_PATH=1 {{ruff_file}} check --fix a.py --output-format=concise --config {{config_file}}

    test-rel:
        @RUFF_SHOW_FULL_PATH=0 {{ruff_file}} check --fix a.py --output-format=concise --config {{config_file}}

----------------------------------------

(venv) d:\tests>just test-rel
a.py:1:4: SyntaxError: Simple statements must be separated by newlines or semicolons
Found 1 error.
error: Recipe `test-rel` failed on line 8 with exit code 1

(venv) d:\tests>just test-abs
d:\tests\a.py:1:4: SyntaxError: Simple statements must be separated by newlines or semicolons
Found 1 error.
error: Recipe `test-abs` failed on line 5 with exit code 1

@MichaReiser MichaReiser added needs-info More information is needed from the issue author and removed cli Related to the command-line interface wish Not on the current roadmap; maybe in the future labels Dec 16, 2024
brupelo pushed a commit to brupelo/ruff that referenced this issue Dec 16, 2024
brupelo pushed a commit to brupelo/ruff that referenced this issue Dec 16, 2024
brupelo pushed a commit to brupelo/ruff that referenced this issue Dec 17, 2024
brupelo pushed a commit to brupelo/ruff that referenced this issue Jan 5, 2025
brupelo pushed a commit to brupelo/ruff that referenced this issue Jan 18, 2025
@brupelo
Copy link
Author

brupelo commented Jan 18, 2025

@MichaReiser Hi,

Out of curiosity, what does the needs-info label mean in this context? I’ve been using this branch successfully for the past month without any issues. I’ve also been rebasing it consistently against the latest origin/main as it got stagnated. However, I’m unsure if this will ever make it to master.

I’d like to help finalize this for release. While my time is limited, I could allocate some if only minor reviews are needed. Could you clarify the bare minimum requirements for a feature to be merged into master? I’ve made efforts to ensure this doesn’t introduce any breaking changes, but I’d like to know what the next steps are.

Thanks!

P.S. The other issue I reported—where adding a lot of imports causes it to get stuck—is also a blocker preventing me from fully leveraging Ruff, that one doesn't seem like a good first-issue though. Other than these, the tool is truly fantastic.

@MichaReiser
Copy link
Member

Needs info means that we need more information from the author, you, to move the issue forward.

To me it's still unclear what you need this feature for. Can you tell me a specific use case/example?

brupelo pushed a commit to brupelo/ruff that referenced this issue Jan 19, 2025
@brupelo
Copy link
Author

brupelo commented Jan 19, 2025

@MichaReiser Hey Micha, sure! I thought I had explained this use-case a few months ago already, and again when I opened this one last month. But let me try again, okay?

I’m using Sublime Text, and I’ve set up some custom commands for running ruff. Here’s how it works: I can either lint a bunch of Python folders (which may not even be in the same workspace, as they include editable packages and subdependencies from different companies or organizations), or I can pick individual Python files from different drives via the sidebar menu.

Of course, the more typical and frequent use case would be running a command with a shortcut while editing a single file or on save. That’s what I usually do when coding and focusing on single tasks. These two workflows—batch linting multiple folders or linting individual files—are things I deal with on a daily basis.

To make this work, I’ve set up some custom commands that hook into ruff, and I pass the corresponding paths. I can show you an example of a typical command so you’ll understand how it looks.

        {
            "command": "zebra_exec_run",
            "keys": ["ctrl+shift+8"],
            "args": {
                "cmd": "ruff check --fix ${file_name} --output-format=concise && ruff format ${file_name}",
                "file_regex": "(.*?\\.py):(.*?):(.*?):(.*)",
                "quiet": true,
                "working_dir": "${directory}",
                "env": {
                    "RUFF_SHOW_FULL_PATH": "1"
                }
            },
            "context": [{"key": "selector", "operator": "equal", "operand": "(source.python)", "match_all": true} ]
        },
        {
            "command": "zebra_exec_run",
            "keys": ["ctrl+shift+9"],
            "args": {
                "cmd": "ruff check --fix ${file_name} --output-format=concise && ruff format ${file_name}",
                "file_regex": "(.*?\\.py):(.*?):(.*?):(.*)",
                "quiet": true,
                "working_dir": "${directory}",
                "env": {
                    "RUFF_SHOW_FULL_PATH": "0"
                }
            },
            "context": [{"key": "selector", "operator": "equal", "operand": "(source.python)", "match_all": true} ]
        },

the above would be simple commands showcasing ruff with absolute paths and with not (i've used my commit rebased on latest origin/main)

but when i need to lint several folders from a single codebase or multiple ones I'd have something like this

                {
                    "caption": "Lint",
                    "command": "zebra_exec_run",
                    "args": {
                        "cmd": "ruff check --fix ${paths} --output-format=concise && ruff format ${paths}",
                        "file_regex": "(.*?\\.py):(.*?):(.*?):(.*)",
                        "quiet": true,
                        "working_dir": "${directory}",
                        "env": {
                            "RUFF_SHOW_FULL_PATH": "1"
                        },
                        "paths": []
                    },
                    "context": [{"key": "selector", "operator": "equal", "operand": "(source.python)", "match_all": true} ]
                },

which i trigger from here

Image

The reason I insisted on this change and spent time working on it is that, with other linters providing absolute paths, I can easily use regex to fix the source code, double-click to navigate, and go to files directly. Without absolute paths, this was impossible in those scenarios.

Does that make sense now? I hope this can make it to the master branch somehow. As I mentioned, I’ve implemented it in a way that wouldn’t break anything for others, but it covers these additional cases. That said, it’s quite a pain to keep rebasing frequently to ensure it stays in sync with the latest origin/main.

I’d really like to be able to check out the latest origin/main without worrying about spending extra time resolving potential merge conflicts or bugs. The good news is that every time I’ve rebased my small diff over the past month, it hasn’t conflicted at all and has worked out of the box. It seems the impacted files are rarely changed these days, which is reassuring.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-info More information is needed from the issue author
Projects
None yet
Development

No branches or pull requests

2 participants