kyu08 / fzf-make

A command line tool that executes make target using fuzzy finder with preview window.
MIT License
77 stars 10 forks source link

Add define block skip #241

Closed Sigmanificient closed 6 months ago

Sigmanificient commented 6 months ago

Hi, I tried to implement the define block skip myself, as it is a feature that I would like to have.

[!NOTE] With some background in C, I'm still unfamiliar with Rust, so I focused on a simple implementation that works. It might not be fancy, and not be the best way to pull this off, so I rely on your expertise for some feedback.

kyu08 commented 6 months ago

@Sigmanificient Thank you for exploring and dissecting the code, and for the effort to submit a PR despite not being familiar with Rust! ☺️ I will aim to respond within the next few days. Just hold on a bit! 🙇‍♂️

Sigmanificient commented 6 months ago

Including both may add quite some complexity to the initial parsing function. So, in an effort to keep readability, maybe I should try to extract a get_line_type function

enum LineType { Normal, DefineStart, DefineEnd }

fn get_line_type(line: &str) -> LineType {
    let words: Vec<&str> = line.split_whitespace().collect();

    if (words.len() >= 2
        && words[0] == OVERRIDE
        && words[1] == DEFINE_BLOCK_START) {
        return LineType::DefineStart;
    }

    match line.trim() {
        DEFINE_BLOCK_START => LineType::DefineStart,
        DEFINE_BLOCK_END => LineType::DefineEnd,
        _ => LineType::Normal,
    }
}

Something like this?

Sigmanificient commented 6 months ago

About the unit testing, shouldn't they be placed in their own function define_block_skip_test?

Sigmanificient commented 6 months ago

Hello again, I updated the source according to the change you requested!

I also made sure to format it properly, adding the required rustfmt to the dev shell in the way. Then I finally added the unit tests, along some whitespace one, just in case. Had to put #[derive(Debug, PartialEq)] macros above the Enum to make it work.

Hopefully it will be good this time!

kyu08 commented 6 months ago

@Sigmanificient This fix released v0.27.0. https://github.com/kyu08/fzf-make/releases/tag/v0.27.0

Sigmanificient commented 6 months ago

Splendid! Just pr-ed the version bump to nixpkgs :eyes:

https://github.com/NixOS/nixpkgs/pull/301279