[Fix/Feature] Restructured The Way Adding Display Opts Works | Added The Structure For A Sharable Command Feature

[KMastroluca]

Fix

Before, when you tried to add a URL, and then add a second URL, literally you would have 2 URLs rendered next to each other in the Options window. That didnt make sense to me, so I restructured some key components of the options adding operation.

The following is the new add_display_option() function.

    pub fn add_display_option(
        &mut self,
        mut opt: DisplayOpts, /* We Make This Mutable Because We Need To Compare It To A Mutable Reference Later */
    ) {
        if self.should_replace_or_add(opt.clone()) {
            // TRUE = We Should Add An Option
            // Adding The New Test Here. should_replace_or_add
            // The user has not yet added this command option yet, and so therefore, we push it to the opts vector.
            // I need a copy of the option before we move it
            let match_opt = opt.clone(); // Lets refactor all this later.
            self.opts.push(opt);
            // We also need to add it to the sharable command
            // but we need to know what kind of option it is
            // the best way I can think of right now is to match it but I'm sure there's a better way
            match match_opt {
                // Im cloning this to shut the borrow checker up, there is probably a better way
                DisplayOpts::Verbose => {
                    // Just add the verbose flag to the command
                    self.shareable_command.as_mut().unwrap().set_verbose(true);
                }
                DisplayOpts::Headers((key, value)) => {
                    // Push Header To Shareable Command
                    self.shareable_command
                        .as_mut()
                        .unwrap()
                        .push_header((key, value));
                }
                DisplayOpts::URL(url) => {
                    // Set URL To Sharable Command
                    self.shareable_command.as_mut().unwrap().set_url(url);
                }
                DisplayOpts::Outfile(outfile) => {
                    // Set Outfile To Shareable Command
                    self.shareable_command
                        .as_mut()
                        .unwrap()
                        .set_outfile(outfile);
                }
                _ => {
                    // Nothing
                    // This display opt does not factor into the sharable command.
                }
            }
        } else {
            // FALSE = We Should Replace An Option
            // The user has already added this command option, and so therefore, we should replace the old value with the new value.
            //self.opts.retain(|x| x != &opt);
            // Sorry, this is my way to do this idk if its the right way, but this is what makes sense to me in my head
            for element in self.opts.iter_mut() {
                // Same thing down here, I only care if its the same KIND of option, not the same value
                // Again, this is annoying, I tried to do this an easier way
                // but mem::discriminant doesnt like element as a comparison so I need to be particular
                // Sorry lets refactor this
                // TODO: Refactor This.

                // We Want To Just Replace A URL
                if let DisplayOpts::URL(_) = element {
                    *element = opt.clone(); // Copy The New URL Into The Old One
                    return;
                }

                // TODO: Headers Will Be Handled Differently.

                // TODO: Outfile Will Be Handled Differently.

                // TODO: Verbose & Save Command Will Be Handled Differently.

                // TODO: Other Shit
            }
        }
    }

It took me a while to figure out that, a lot of the ways that we were testing if options had already been added was kinda broken. It could be that I just didnt understand it, but It didnt make sense to me, and so to make some actual progress, I just implemented a process that made sense to me.

These are the new tests for if we have a given option in the option vector.

    // Display option is some state that requires us to display the users
    // current selection on the screen so they know what they have selected
    // Lorenzo - Changing this because I dont think its doing what I want it to do.
    pub fn has_display_option(&self, opt: DisplayOpts) -> bool {
        // this is what we were doing before - self.opts.iter().any(|x| &x == &&opt)
        // I think this is what we want to do instead
        // We want to check if the option is in the vector
        // If it is, we want to return true
        // If it is not, we want to return false
        // We can do this by iterating over the vector and checking if the option is in the vector
        for element in self.opts.iter() {
            // I only care if its the same KIND of option, not the same value
            // This is annoying, I tried to do this an easier way
            // but mem::discriminant doesnt like element as a comparison so I need to be particular
            // im sorry i really dont like this
            // TODO: Lets refactor this.
            if let DisplayOpts::URL(_) = *element {
                return true;
            }

            if let DisplayOpts::Headers(_) = *element {
                return true;
            }

            if let DisplayOpts::Outfile(_) = *element {
                return true;
            }

            if let DisplayOpts::Verbose = *element {
                return true;
            }

            if let DisplayOpts::SaveCommand = *element {
                return true;
            }

            if let DisplayOpts::Response(_) = *element {
                return true;
            }

            if let DisplayOpts::ShareableCmd(_) = *element {
                return true;
            }
        }
        // Otherwise, its not there.
        false
    }

    // Lorenzo - Im adding this function as a slightly more
    // robust version of has_display_option, to test if we should be replacing a value or adding a new one
    fn should_replace_or_add(&self, opt: DisplayOpts) -> bool {
        // Lets match the type of display option
        // We know that only 1 URL should ever be added,
        // So if we're adding a URL we should replace it if it already exists
        match opt {
            DisplayOpts::URL(_) => !self.has_display_option(opt.clone()), // URL Should Be Replaced If It Already Exists
            DisplayOpts::Headers(_) => true, // Headers Should Be "Pushed" or Added
            DisplayOpts::Outfile(_) => !self.has_display_option(opt.clone()), // Outfile Should Be Replaced
            DisplayOpts::Verbose => !self.has_display_option(opt.clone()), // Verbose Should Be Toggled
            DisplayOpts::SaveCommand => !self.has_display_option(opt.clone()), // Save Command Should Be Toggled
            DisplayOpts::Response(_) => !self.has_display_option(opt.clone()), // Response Should Be Replaced
            DisplayOpts::ShareableCmd(_) => !self.has_display_option(opt.clone()), // Shareable Command Should Be Replaced With The New Version
            _ => false, // Anything Else Should Be Replaced.
        }
    }

My comments sort of explain myself as I go. I tried to do variant comparison with a function called std::mem::discriminant() but for whatever reason, it didnt like the element variable from self.opts.iter()? So this is kinda my hacky fix. I know theres a better way to do this, but for now, its verbose and clear what I mean. But its also kinda redundant and Im sure theres an easier way to do the same thing.

---

As you can see, ive also added additional functionality for the sharable curl command. That has been implemented as follows, but ive yet to touch any UI code having anything to do that yet, so basically, the data structure is there, but I havent thoroughly tested it yet. It shouldnt interfere with the operation of the application tho.

Sharable Command Structure / Implementation

impl ShareableCommand {
    pub fn new() -> Self {
        Self {
            command: "".to_string(),
            url: "".to_string(),
            headers: Vec::new(),
            outfile: "".to_string(),
            verbose: false,
        }
    }

    pub fn set_command(&mut self, command: String) {
        self.command = command;
    }

    pub fn set_verbose(&mut self, verbose: bool) {
        self.verbose = verbose;
    }

    pub fn set_url(&mut self, url: String) {
        self.url = url;
    }

    pub fn set_headers(&mut self, headers: Vec<(String, String)>) {
        self.headers = headers;
    }

    pub fn push_header(&mut self, header: (String, String)) {
        self.headers.push(header);
    }

    pub fn set_outfile(&mut self, outfile: String) {
        self.outfile = outfile;
    }

    pub fn render_command_str(&self) -> Option<String> {
        if self.command.is_empty() {
            return None;
        }

        if self.url.is_empty() {
            return None;
        }

        // This assembles the simplest possible command string
        let mut command_str = self.command.clone();

        // Check For Verbose Flag
        if self.verbose {
            // Verbose Flag Including Whitespace
            command_str.push_str(" -v");
        }

        // Whitespace
        command_str.push_str(" ");
        // URL
        command_str.push_str(&self.url);

        // Next We Check For Headers
        if self.headers.len() > 0 {
            for (key, value) in &self.headers {
                // Whitespace
                command_str.push_str("");
                // Header Flag
                command_str.push_str("-H ");
                // Open Quote
                command_str.push_str("\"");
                // Header Key
                command_str.push_str(&key);
                // Delimiter
                command_str.push_str(":");
                // Header Value
                command_str.push_str(&value);
                // Close Quote
                command_str.push_str("\"");
            }
        }

        // Check For Outfile
        if !self.outfile.is_empty() {
            // Whitespace
            command_str.push_str(" ");
            // Outfile Flag
            command_str.push_str(" -o ");
            // Outfile Name
            command_str.push_str(&self.outfile);
        }

        // Return Command String
        Some(command_str)
    }
}

Ill make a note that this structure is somewhat similar to your existing Command structure, but it has the curl command string formatting function, which I imagine will be getting plenty more complicated as we add new features.

[KMastroluca]

Please dont merge this yet i just saw a really dumb mistake

[KMastroluca]

That last commit should have fixed it, but maybe im crazy and it wasnt broken anyway idk. Now im confident it works.

[KMastroluca]

Should be good.

[PThorpe92]

NVM: I'll fix the conflicts. Looks good man

Yo I fucking commented:
Hey can you fix these merge conflicts? Here in this comment,
then i edited it and wrote. NVM: I'lll fix the conflicts. Looks good man ^^^ as above.

Somehow that made it into the fucking commit dude. look at 209fd9d

[PThorpe92]

Lol this is why testing... That little slip up would have got merged into main and gone unnoticed (till someone compiled locally but still)

From now on, tests have to be included for every feature at the bottom of the respective file. if there becomes too many, we'll have to create a whole dir

[KMastroluca]

Yeah i was confused. Lol, I'm doing a little abstraction and moving things around into modules because app.rs is getting dangerously long.

[PThorpe92]

Ok feel free to create a tests dir and put the tests in there. Or if you want to split that up to 2 diff files. it's up to you, i'll review it and if i dont like it we can do something else :)