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

feat: add config field to show names. resolves #7 #8

Merged
merged 6 commits into from
Jan 27, 2024

Conversation

vague2k
Copy link
Contributor

@vague2k vague2k commented Jan 26, 2024

What does this PR do?

  • Add a config field called show_project_names which is a boolean
  • Checks in telescope and vim-ui adapters to check if the show_project_names is true

Telescope implementation

Fairly straight forward
Add a check in pickers.finder.entry_maker field in telescope.lua to see if show_project_names is true
If it is true, display = project.name

Vim.ui implementation

A bit tricky, i left a HACK: comment
We pass an array of project names to show as a list of possible choices. We then loop over all json objects (projects) in the json file and if the choice matches the name property in the object, we use the objects path property as the value to pass to api.cd_project

@LintaoAmons
Copy link
Owner

I don't want that option, if we have the name, we just display both the name and the path~
You can have a look, if no other changes, I will merge

@vague2k
Copy link
Contributor Author

vague2k commented Jan 26, 2024

I personally thinking an option to show either just the name or just the path, or even both like how you implemented is worth it. But personal opinions aside, looks good to me!

@LintaoAmons
Copy link
Owner

Sound good~

Can you help to add it? I hope to encapsulate this logic in a function, with a signature similar to formEntry(project, max_len) -> string.

@vague2k
Copy link
Contributor Author

vague2k commented Jan 27, 2024

Is it fine to add another config field? I can't think of a way to distinguish between explicitly showing either paths or names or both without one

@LintaoAmons
Copy link
Owner

Yeah, I think it's fine to add another optional config field~

@vague2k
Copy link
Contributor Author

vague2k commented Jan 27, 2024

Just implemented. Works fine when I tested. let me know if any feedback

@LintaoAmons LintaoAmons merged commit 046cb76 into LintaoAmons:main Jan 27, 2024
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.

2 participants