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 tab completion to Add sample file dir dialog #276

Merged
merged 4 commits into from Jul 4, 2023

Conversation

sky1e
Copy link
Contributor

@sky1e sky1e commented Jul 2, 2023

This ended up significantly more involved to implement than I expected when I started.

image

On the Add sample file directory screen, implement path tab completion. I ended up having to create custom views for the behavior, which wrap existing views, but change their behavior with a couple events.

Copy link
Owner

@scottlamb scottlamb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is really neat. I left a couple minor comments.

FYI, in case you have too many more TUI config improvements in mind, I should warn you: I'd ultimately like to switch everything to web-based config (#35).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • This is really neat. I wonder if the cursive folks would be interested in a PR to add it to that library.
  • Please copy'n'paste a copyright header (see failing CI action).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I be adding myself to the AUTHORS file as well then? I haven't signed away my copyright, so just copy-pasting the copyright header would be inaccurate.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, 👍 welcome to the Moonfire author club!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alternatively, the notice could be updated to say "Copyright (C) 2017 The Moonfire NVR Authors and contributors"

server/src/cmds/config/dirs.rs Outdated Show resolved Hide resolved
@sky1e
Copy link
Contributor Author

sky1e commented Jul 4, 2023

FYI, in case you have too many more TUI config improvements in mind, I should warn you: I'd ultimately like to switch everything to web-based config (#35).

I was presuming that even with a decent web ui, the TUI wouldn't just be removed. There are advantages to having a UI that requires nothing but terminal access to the server, as an option.

@scottlamb
Copy link
Owner

I was presuming that even with a decent web ui, the TUI wouldn't just be removed. There are advantages to having a UI that requires nothing but terminal access to the server, as an option.

I'll tag you in on #35 and continue this conversation there.

@scottlamb scottlamb mentioned this pull request Jul 4, 2023
@scottlamb scottlamb merged commit c2d226d into scottlamb:master Jul 4, 2023
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants