feat: add capability to create new entries with alt+n uses clipboard for url that can be overridden #116
Conversation
…for url that can be overridden
There was a problem hiding this comment.
Pull Request Overview
This PR adds a new capability to create entries directly from the rofi-rbw interface using the Alt+n keyboard shortcut. The feature attempts to pre-fill URL information from the clipboard and guides users through creating new password entries with generated secure passwords.
- Added
ADDaction to the Action enum and integrated it into the main workflow - Implemented
show_input_dialogmethod across all selector implementations (rofi, wofi, fuzzel, bemenu) - Added
read_from_clipboardfunctionality to all clipboarder implementations - Created entry creation workflow that extracts domain names from URLs and generates passwords
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/rofi_rbw/models/action.py | Adds ADD action to enum |
| src/rofi_rbw/argument_parsing.py | Registers Alt+n keyboard shortcut for add action |
| src/rofi_rbw/rofi_rbw.py | Implements main entry creation workflow and URL handling |
| src/rofi_rbw/rbw.py | Adds password generation and entry creation methods |
| src/rofi_rbw/abstractionhelper.py | Adds URL domain extraction utility function |
| src/rofi_rbw/selector/*.py | Implements input dialog functionality for all selectors |
| src/rofi_rbw/clipboarder/*.py | Adds clipboard reading capability to all clipboarder implementations |
| docs/rofi-rbw.1.md | Updates documentation with new add functionality |
| README.md | Updates README with Alt+n feature description |
Comments suppressed due to low confidence (1)
src/rofi_rbw/selector/selector.py:1
- The import statement for 're' module is being removed but the module is still used in the _format_further_item_name method on line 127. This will cause a NameError at runtime.
from abc import ABC, abstractmethod
| targets.append(f"{self._format_further_item_name(field.key)}: {field.masked_string()}") | ||
|
|
||
| notes_text = note.notes.replace('\n', '|') | ||
| targets.append(f"Notes: {notes_text}") |
There was a problem hiding this comment.
The loop that processes note.fields has been removed, but the method should still handle custom fields in notes. This change may cause custom fields to not be displayed.
| targets.append(f"Notes: {notes_text}") | |
| targets.append(f"Notes: {notes_text}") | |
| for field in getattr(note, "fields", []): | |
| targets.append(f"{self._format_further_item_name(field.key)}: {field.masked_string()}") |
Copilot uses AI. Check for mistakes.
| # Check if clipboard contains a URL | ||
| if clipboard_content.startswith(('http://', 'https://')) or ('.' in clipboard_content and not clipboard_content.isspace()): | ||
| clipboard_url = clipboard_content if clipboard_content.startswith(('http://', 'https://')) else f"https://{clipboard_content}" |
There was a problem hiding this comment.
The URL detection logic is overly broad and could match non-URL strings. A string like 'my.file' or 'john.doe' would be treated as a URL, which could lead to incorrect behavior.
| # Check if clipboard contains a URL | |
| if clipboard_content.startswith(('http://', 'https://')) or ('.' in clipboard_content and not clipboard_content.isspace()): | |
| clipboard_url = clipboard_content if clipboard_content.startswith(('http://', 'https://')) else f"https://{clipboard_content}" | |
| # Check if clipboard contains a valid URL | |
| parsed = urlparse(clipboard_content) | |
| if (parsed.scheme in ('http', 'https') and parsed.netloc) or (not parsed.scheme and '.' in clipboard_content and not clipboard_content.isspace()): | |
| # If it already has a scheme and netloc, use as is; otherwise, prepend https:// and check again | |
| if parsed.scheme in ('http', 'https') and parsed.netloc: | |
| clipboard_url = clipboard_content | |
| else: | |
| # Try parsing with https:// prepended | |
| parsed2 = urlparse(f"https://{clipboard_content}") | |
| if parsed2.scheme == 'https' and parsed2.netloc: | |
| clipboard_url = f"https://{clipboard_content}" |
Copilot uses AI. Check for mistakes.
|
|
||
| # Check if clipboard contains a URL | ||
| if clipboard_content.startswith(('http://', 'https://')) or ('.' in clipboard_content and not clipboard_content.isspace()): | ||
| clipboard_url = clipboard_content if clipboard_content.startswith(('http://', 'https://')) else f"https://{clipboard_content}" |
There was a problem hiding this comment.
This line automatically prefixes 'https://' to any content that contains a dot but doesn't start with a protocol. This could incorrectly convert non-URL strings like filenames or email addresses into invalid URLs.
| # Check if clipboard contains a URL | |
| if clipboard_content.startswith(('http://', 'https://')) or ('.' in clipboard_content and not clipboard_content.isspace()): | |
| clipboard_url = clipboard_content if clipboard_content.startswith(('http://', 'https://')) else f"https://{clipboard_content}" | |
| # Check if clipboard contains a URL or a plausible domain (but not an email or file path) | |
| if clipboard_content.startswith(('http://', 'https://')): | |
| clipboard_url = clipboard_content | |
| else: | |
| # Exclude email addresses and file paths | |
| is_email = re.match(r"^[^@]+@[^@]+\.[^@]+$", clipboard_content) | |
| is_file_path = re.match(r"^([a-zA-Z]:)?[\\/]", clipboard_content) or clipboard_content.startswith(('.', '/')) | |
| # Check for plausible domain (contains a dot, no spaces, not email, not file path) | |
| is_domain = ( | |
| '.' in clipboard_content | |
| and not clipboard_content.isspace() | |
| and not is_email | |
| and not is_file_path | |
| and re.match(r"^[a-zA-Z0-9.-]+$", clipboard_content) | |
| ) | |
| if is_domain: | |
| clipboard_url = f"https://{clipboard_content}" |
Copilot uses AI. Check for mistakes.
|
|
||
| def extract_domain_from_url(url: str) -> str: | ||
| """Extract domain from URL, removing protocol, www, and path/query parameters.""" | ||
| import re |
There was a problem hiding this comment.
The function imports the 're' module inside the function body. It would be more efficient and conventional to import 're' at the module level.
| import re |
Copilot uses AI. Check for mistakes.
| def sync(self): | ||
| run(["rbw", "sync"]) | ||
|
|
||
| def generate_password(self, length: int = 16) -> str: |
There was a problem hiding this comment.
The generate_password method is defined but never used in the codebase. The add_entry method directly calls 'rbw generate' instead of using this method, creating duplicate functionality.
Copilot uses AI. Check for mistakes.
|
Hey, thank you for not simply opening an issue but already having a suggestion for a solution 🙂 To be honest, I have to think about whether this is something I want in Finally, I feel that I have to comment on the vibe-coding thing. I'm not opposed to it, but I do expect the same quality from an AI as from a human developer. In this PR, a lot has changed, and in some cases, I can't see the reasoning. For example, an I'll think about whether I want the functionality, and if so, we'll work through the code together 🙂 |
|
Hey there, thanks for the comments. I understand, I've just gotten used to this in my previous workflow. I cobbled together a simple bash script that used gopass. With that in mind I just thought I'd share this feature here, and I'm happy to clean up a little and am glad for any assistance if this is something that you'd consider merging. The next feature (and last one) would be a password rotation command (regen), when the website you just logged in nags you with "you gotta change your password". With those features there is no need to use any other client 🤷 so that might be an argument 😄 In any case, thanks for the great work on this. |
|
I definitely see the appeal of this. Currently, I use the web interface for any editing, so integrating it would be kinda nice. But again, thanks for the ideas and the draft 🙂 |
2 participants
Hey there and thanks for this great rofi script.
I'm just in the course of migrating from gnu pass to vaultwarden and really am liking things so far.
This rofi script kind of is what I had with pass, but it is lacking an add new entry feature.
I'm not super fluent with python so disclaimer: I've vibe coded this feature. It seems to be doing what I want so I think this could be something that others like as well.
usage:
Copy a url (or not), bring up rofi-rbw and hit
alt+nthe url will be prefilled (you can override this value), you can then provide a username. After that a password is generated and added to the clipboard ready to paste.