GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Move recent files item to new context menu and add clear recent file function by lucydodo · Pull Request #2347 · sqlitebrowser/sqlitebrowser

Conversation

@lucydodo

@lucydodo lucydodo commented Jul 28, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Recently used files in DB4S were previously displayed at the bottom of the file menu.
However, I've implemented this idea because I thought it would be better to create a menu called Recent Files in the File menu, and access the recent files in it and delete the list if necessary.

and also, I'm ready to listen to any advice. This is because I'm NOT sure about the QT framework and I'm afraid about requesting this PR without knowing the atmosphere of the DB4S team to see if it is okay to send a contribution to the implementation of the function. I wait for your valuable advice or feedback. Thanks!

Known bugs

  • After opening the file, it is not added to the recent files list (but added normally when the DB4S is restarted)
  • I cross-validated with the nightly build (release on July 27, 2020) to confirm that is is a problem in my code.
    • MD5 : e1d4952fd91625403c7a57ca7be3e252
    • SHA256 : 53739c57b44c02fea06758eb6e95be12598c54da5f9700c52d48ff64d8ca90ba

Test Environment

  • macOS Catalina 10.15.6
  • Homebrew 2.4.9

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
SeongTae Jeong added 5 commits July 29, 2020 05:00
SeongTae Jeong
Add a new menu item to the list of recent files in the file menu
71c0bf2
SeongTae Jeong
Declare method prototype that delete recent file list
af07ebe
SeongTae Jeong
Implement method that delete recent file list
a55d368
SeongTae Jeong
Declare a method prototype to handle the newly added menu item
4eacb91 
Declare a method prototype to handle the newly added menu item.
and delete divider(QAction *recentSeparatorAct) because that is no longer used.
SeongTae Jeong
Implement method that delete recent file list
3880ad9 
Implement method that delete recent file list by newly added menu item.
and delete divider(QAction *recentSeparatorAct) because that is no longer used.
@justinclift

justinclift commented Jul 28, 2020

Copy link
Copy Markdown
Member

Seems like a reasonable idea. 😄

Btw, there's a typo in your description of the PR: Recent Lists -> Recent Files

The source code doesn't have the typo though. 😉
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@lucydodo

lucydodo commented Jul 28, 2020

Copy link
Copy Markdown
Member Author

I just fixed it. Thank you!
justinclift reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@justinclift justinclift added this to the 3.13.0 - Future release milestone Jul 28, 2020
@justinclift justinclift added the enhancement Feature requests. label Jul 28, 2020
@lucydodo

lucydodo commented Jul 28, 2020

Copy link
Copy Markdown
Member Author

May I close the issue without further discussion?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@justinclift

justinclift commented Jul 28, 2020

Copy link
Copy Markdown
Member

Technically yes. First through, it's probably a better idea to let people know why you want to close it, just in case there's a better way. 😄
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@lucydodo

lucydodo commented Jul 28, 2020

Copy link
Copy Markdown
Member Author

There is no particular reason to close it. I will keep it for the time being for further better comments. :D
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
mgrojo
mgrojo requested changes Jul 28, 2020
View reviewed changes

@mgrojo mgrojo left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks for this contribution, @lucydodo! I've made a review and if you address at least the main problem (actions should be hidden and not removed), I think it can be added to the master branch.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/MainWindow.cpp Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/Settings.cpp Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread src/MainWindow.cpp Resolved

Uh oh!

There was an error while loading. Please reload this page.
SeongTae Jeong added 6 commits July 29, 2020 08:27
SeongTae Jeong
Rename 'Clear Menu' to 'Clear List'
6bdcb7d 
A mgrojo comment that "Clear List" is better than "Clear Menu" corrects. [#2347]
SeongTae Jeong
Update function to general function
fa3511d
SeongTae Jeong
Revert "Update function to general function"
eaa8d7e 
This reverts commit fa3511d.
SeongTae Jeong
Update function to general function
ec887df
SeongTae Jeong
Change the action to hide instead of delete
bda027c
SeongTae Jeong
Clear cache too when claerValue() method called
0b3869e
@lucydodo

lucydodo commented Aug 1, 2020

Copy link
Copy Markdown
Member Author

@mgrojo I have modified the code according to your review.
Please review it and give me another request if necessary. Thank you!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mgrojo
mgrojo merged commit ec6ff6a into sqlitebrowser:master Aug 1, 2020
@lucydodo
lucydodo deleted the dev-recent_file_menus branch August 1, 2020 16:08
@mgrojo

mgrojo commented Aug 1, 2020

Copy link
Copy Markdown
Member

@lucydodo Nice work! It's now squashed and merged. Thank you! The list of recent files is now in line with current standards and it doesn't make the File menu so wide. Maybe we could increase now the number of recent files to 9, for example.
justinclift reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@lucydodo

lucydodo commented Aug 1, 2020

Copy link
Copy Markdown
Member Author

@mgrojo Thank you for merge, I couldn't be happier about it!
I think your idea is nice so I will try to give the user the option to increase the number of recent files list.
justinclift reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mgrojo

mgrojo commented Aug 1, 2020

Copy link
Copy Markdown
Member

I think your idea is nice so I will try to give the user the option to increase the number of recent files list.

I simply meant to increase the value of MaxRecentFiles to get more entries in the list, but you are free to implement a preference for that.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@lucydodo lucydodo mentioned this pull request Aug 15, 2020
Allow the user can change MaxRecentFiles #2373
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mgrojo mgrojo mgrojo requested changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

enhancement Feature requests.

Projects

None yet

Milestone

3.13.0 - Future release

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@lucydodo @justinclift @mgrojo
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.