numtide/treefmt
Archived
8
0
Fork 0
Code Issues 1 Pull Requests Packages Projects Releases Wiki Activity

Feature parity with treefmt.rs #22

Merged
zimbatm merged 7 commits from feat/explicit-paths-and-stdin into main 2024-02-19 09:54:58 +00:00
Conversation 1 Commits 7 Files Changed 27 +450 -177
brianmcgee commented 2024-01-12 08:50:10 +00:00
Member
Copy Link

Introduces remaining features which should bring us into parity with treefmt.rs.

  • accepts a list of explicit path arguments for formatting or pulls them from stdin.
  • remove the internal directory, favouring a flatter repo structure
  • support --version
  • support --no-cache
Introduces remaining features which should bring us into parity with [treefmt.rs](https://github.com/numtide/treefmt). * accepts a list of explicit path arguments for formatting or pulls them from stdin. * remove the internal directory, favouring a flatter repo structure * support `--version` * support `--no-cache`
brianmcgee added 2 commits 2024-01-12 08:50:10 +00:00 
Signed-off-by: Brian McGee <brian@bmcgee.ie>
Signed-off-by: Brian McGee <brian@bmcgee.ie>
brianmcgee requested review from zimbatm 2024-01-12 08:50:17 +00:00
zimbatm reviewed 2024-01-12 11:35:52 +00:00
internal/cli/format.go Outdated
@ -196,0 +195,4 @@
if Cli.Stdin {
walker, err = walk.NewPathReader(os.Stdin)
} else if len(Cli.Paths) > 0 {
walker, err = walk.NewPathList(Cli.Paths)
zimbatm commented 2024-01-12 11:35:52 +00:00
Owner

In terms of behaviour, would you expect treefmt ./some/gitignored-file to format or ignore the file?

In the previous implementation, the list of paths would be a selector over the normal walker, excludes and includes list.

In terms of behaviour, would you expect `treefmt ./some/gitignored-file` to format or ignore the file? In the previous implementation, the list of paths would be a selector over the normal walker, excludes and includes list.
brianmcgee commented 2024-01-12 12:30:53 +00:00
Author
Member

Thinking on this, if I explicitly provide a path my expectation would be that treefmt operates on what I told it to, applying the matching rules based on the config. I wouldn't expect it to be applying gitignore logic too. That seems non-intuitive.

You either walk the file system, traverse the git index or operate on exactly what I tell you to operate on.

Is there a good example of why the current behaviour is necessary?

Thinking on this, if I explicitly provide a path my expectation would be that `treefmt` operates on what I told it to, applying the matching rules based on the config. I wouldn't expect it to be applying gitignore logic too. That seems non-intuitive. You either walk the file system, traverse the git index or operate on exactly what I tell you to operate on. Is there a good example of why the current behaviour is necessary?
zimbatm commented 2024-01-13 19:22:29 +00:00
Owner

Imagine you want to integrate treefmt with your editor. For example, format the file on save. How does the editor know which files should be formatted? I assume it would just call treefmt <buffer_file_name> and expect treefmt to make the decision.

Imagine you want to integrate treefmt with your editor. For example, format the file on save. How does the editor know which files should be formatted? I assume it would just call `treefmt <buffer_file_name>` and expect treefmt to make the decision.
brianmcgee commented 2024-01-15 11:00:43 +00:00
Author
Member

🤔 so walk as usual, but filter any paths which don't match the provided paths? In the case of a regular file it's simple, if a directory is provided then it's a prefix check.

🤔 so walk as usual, but filter any paths which don't match the provided paths? In the case of a regular file it's simple, if a directory is provided then it's a prefix check.
zimbatm commented 2024-01-16 10:54:41 +00:00
Owner

How about passing the list of files to the walker?

$ git ls-files flake.nix doesnt-exists
flake.nix
How about passing the list of files to the walker? ``` $ git ls-files flake.nix doesnt-exists flake.nix ```
brianmcgee marked this conversation as resolved
brianmcgee force-pushed feat/explicit-paths-and-stdin from f7ad34ca68 to 557b206227 2024-01-12 12:27:31 +00:00 Compare
brianmcgee force-pushed feat/explicit-paths-and-stdin from 557b206227 to 20b23a5225 2024-01-15 11:07:25 +00:00 Compare
brianmcgee force-pushed feat/explicit-paths-and-stdin from 20b23a5225 to 1f55163b93 2024-01-15 11:08:41 +00:00 Compare
brianmcgee added 1 commit 2024-01-15 11:37:38 +00:00 
Signed-off-by: Brian McGee <brian@bmcgee.ie>
brianmcgee changed title from feat/explicit-paths-and-stdin to WIP: feat/explicit-paths-and-stdin 2024-01-15 11:37:49 +00:00
brianmcgee force-pushed feat/explicit-paths-and-stdin from 09e81bdd85 to 5e4d61d6d6 2024-02-14 16:54:16 +00:00 Compare
brianmcgee commented 2024-02-14 16:58:00 +00:00
Author
Member
Copy Link

@zimbatm followed your approach with passing the path to the walker. I updated nix fmt to use the package from the flake and it seems to work. Need to add some tests.

@zimbatm followed your approach with passing the path to the walker. I updated `nix fmt` to use the package from the flake and it seems to work. Need to add some tests.
brianmcgee force-pushed feat/explicit-paths-and-stdin from 5e4d61d6d6 to 72bd5960a7 2024-02-15 10:39:05 +00:00 Compare
brianmcgee changed title from WIP: feat/explicit-paths-and-stdin to feat/explicit-paths-and-stdin 2024-02-15 10:40:08 +00:00
brianmcgee changed title from feat/explicit-paths-and-stdin to Feature parity with treefmt.rs 2024-02-15 10:41:34 +00:00
brianmcgee added 1 commit 2024-02-15 10:48:09 +00:00 
Signed-off-by: Brian McGee <brian@bmcgee.ie>
brianmcgee force-pushed feat/explicit-paths-and-stdin from a99b9513be to cb8565d683 2024-02-15 14:00:06 +00:00 Compare
brianmcgee added 1 commit 2024-02-15 14:00:53 +00:00 
Signed-off-by: Brian McGee <brian@bmcgee.ie>
brianmcgee added 1 commit 2024-02-15 14:17:22 +00:00 
Signed-off-by: Brian McGee <brian@bmcgee.ie>
zimbatm approved these changes 2024-02-15 20:58:20 +00:00
zimbatm left a comment
Owner

yep this is great. I need to do some manual testing to get a better sense of the behaviour, but I think this can be merge already.

yep this is great. I need to do some manual testing to get a better sense of the behaviour, but I think this can be merge already.
👍 1
zimbatm merged commit 9de4fd4cf9 into main 2024-02-19 09:54:58 +00:00
zimbatm deleted branch feat/explicit-paths-and-stdin 2024-02-19 09:54:58 +00:00
This repo is archived. You cannot comment on pull requests.
Reviewers
No reviewers
zimbatm
Labels
Clear labels   
Kind/Breaking

Breaking change that won't be backward compatible

  
Kind/Bug

Something is not working

  
Kind/Documentation

Documentation changes

  
Kind/Enhancement

Improve existing functionality

  
Kind/Feature

New functionality

  
Kind/Security

This is security issue

  
Kind/Testing

Issue or pull request related to testing

  
Priority
Critical
The priority is critical

  
Priority
High
The priority is high

  
Priority
Low
The priority is low

  
Priority
Medium
The priority is medium

  
Reviewed
Confirmed
Issue has been confirmed

  
Reviewed
Duplicate
This issue or pull request already exists

  
Reviewed
Invalid
Invalid issue

  
Reviewed
Won't Fix
This issue won't be fixed

  
Status
Abandoned
Somebody has started to work on this but abandoned work

  
Status
Blocked
Something is blocking this issue or pull request

  
Status
Need More Info
Feedback is required to reproduce issue or to continue work

No Label
Kind/Breaking
Kind/Bug
Kind/Documentation
Kind/Enhancement
Kind/Feature
Kind/Security
Kind/Testing
Priority
Critical
Priority
High
Priority
Low
Priority
Medium
Reviewed
Confirmed
Reviewed
Duplicate
Reviewed
Invalid
Reviewed
Won't Fix
Status
Abandoned
Status
Blocked
Status
Need More Info
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: numtide/treefmt#22
No description provided.
Delete Branch "feat/explicit-paths-and-stdin"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?