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

Minor fixes on utility #145

Closed
wants to merge 1 commit into from
Closed

Minor fixes on utility #145

wants to merge 1 commit into from

Conversation

ColinHebert
Copy link
Contributor

  • mkdcd should use cd's completion, as only the first parameter is used
  • no need to force builtins commands, if the user wants to wrap them,
    those wraps should be used

 - mkdcd should use cd's completion, as only the first parameter is used
 - no need to force builtins commands, if the user wants to wrap them,
   those wraps should be used
@sorin-ionescu
Copy link
Owner

The wraps may fail.

@ColinHebert
Copy link
Contributor Author

Wouldn't that be the responsibility of the user?

@sorin-ionescu
Copy link
Owner

No. RVM used to wrap cd, and it caused nothing but problems.

@ColinHebert
Copy link
Contributor Author

Still, this is the user's problem. I would expect cdll to behave exactly as if I actually typed cd followed by ll.

@sorin-ionescu
Copy link
Owner

RVM caused me to prepend builtin in the first place. I am the user.

As for the completion for mkdcd, it should be mkdir's completion, not cd's. The directory does not exist. Using the mkdir completion, the user gets documentation for mkdir -p, which is useful.

@ColinHebert
Copy link
Contributor Author

So how you use cd inside your terminal, every time you time builtin cd instead of just using cd?

Regarding the completion, mkdir's completion also contains the completion for -m -v --help and --version which is misleading because if any option is passed to mkdcd the command will have an erroneous behaviour.

@sorin-ionescu
Copy link
Owner

The proper way to fix this is to check for mkdir options, not to switch to cd's completion, which gives nothing of use but cd -0/Desktop/. What's up with that -0?

@ColinHebert
Copy link
Contributor Author

I think that's the -n option of cd that shows up (actually I didn't thought of that one).

The third form of cd extracts an entry from the directory stack, and changes to that directory. An argument of the form '+n' identifies a stack entry by counting from the left of the list shown by the dirs command, starting with zero. An argument of the form '-n' counts from the right. If the PUSHD_MINUS option is set, the meanings of '+' and '-' in this context are swapped.

@sorin-ionescu
Copy link
Owner

If you want to fix mkdcd, add option parsing then shift by 1 if an option has been detected.

@ColinHebert
Copy link
Contributor Author

What about using _path_files -/ for completion?

@sorin-ionescu
Copy link
Owner

Why? mkdir is the main command here.

@ColinHebert
Copy link
Contributor Author

Because only the first parameter will be used, and this parameter has to be a directory. Even if mkdir is the main command, the only thing that really matters here is that first parameter.

@sorin-ionescu
Copy link
Owner

I disagree.

If you care so much about fixing it, detect mkdir options.

@ColinHebert
Copy link
Contributor Author

You disagree on what? Right now mkdcd only uses one parameter, that's just a fact. And that parameter MUST consist in a path to an existing directory followed by new sub-folders that will be automatically created. I'm not sure what is the thing to agree on here.

The solution I just talked about does exactly that and avoid problems that mkdcd currently have when you use the completion.

And yes of course it's possible to rewrite an entire function that will check which parameters are passed (and if the said parameter is -m handle the fact that there is an associated value) but that's not the point. Actually that's way more than what mkdcd is about!
The options we're talking about here are --version, --verbose, --help and -m, none of them are relevant to the mkdcd command (-m maybe, but I'm not even sure that should be handled at all).

@sorin-ionescu
Copy link
Owner

I do not want to handle options; I was thinking of just passing them to mkdir and let it handle them.

_path_files -/ doesn't work. It completes both files and directories.

@sorin-ionescu
Copy link
Owner

I found a bug. utility depends on alias for the *ll functions.

@sorin-ionescu sorin-ionescu reopened this Apr 15, 2012
@sorin-ionescu
Copy link
Owner

See branch issue-145.

There is a problem though. completion has to be loaded between alias and utility.

One option is to not depend on the ll alias and just use ls -lh instead. I'm using builtins everywhere else anyway.

@ColinHebert
Copy link
Contributor Author

Yes, I think that the dependency on alias isn't that important and ls -lh is an easy way to get rid of this dependency.

But even if we do that, completion may depend on alias ($LSCOLORS), and utility may depend on completion (compdef), so we're still stuck with alias, completion, utility in this order.

@sorin-ionescu
Copy link
Owner

I have removed the alias module dependency.

@sorin-ionescu
Copy link
Owner

How is the utility/init.zsh now? I'd like to merge it.

@sorin-ionescu
Copy link
Owner

I'm merging it. We'll make a new issue when we figure out how to deal with compdef. I wonder if it's possible to add multiple definitions in a _foo file.

sorin-ionescu added a commit that referenced this pull request May 5, 2012
Functions cdll, pushdll, and popdll depend on the ll alias.
sorin-ionescu added a commit that referenced this pull request May 5, 2012
sorin-ionescu added a commit that referenced this pull request May 5, 2012
lildude pushed a commit to lildude/prezto that referenced this pull request Jan 12, 2014
Functions cdll, pushdll, and popdll depend on the ll alias.
lildude pushed a commit to lildude/prezto that referenced this pull request Jan 12, 2014
lildude pushed a commit to lildude/prezto that referenced this pull request Jan 12, 2014
lildude pushed a commit to lildude/prezto that referenced this pull request Jan 12, 2014
lildude pushed a commit to lildude/prezto that referenced this pull request Jan 12, 2014
krig added a commit to krig/prezto that referenced this pull request Oct 24, 2015
* prompt/external/pure 0421252...dec0253 (26):
  > Update oh-my-zsh instructions
  > 1.2.0
  > Bump zsh-async to 1.0.0, prevents mixed stdout/stderr
  > Merge pull request sorin-ionescu#153 from sindresorhus/preprompt-update-fix
  > Merge pull request sorin-ionescu#149 from mafredri/pure-nitro
  > Close sorin-ionescu#147 PR: Preserve preprompt on Ctrl+L. Fixes sorin-ionescu#145
  > Remove cr from prompt_opts, fixes sorin-ionescu#127
  > Merge pull request sorin-ionescu#142 from zmwangx/string-length-fix
  > Merge pull request sorin-ionescu#144 from zmwangx/rename-prompt-to-preprompt
  > Default to 0 for git rev-list left and right. Fixes sorin-ionescu#137.
  > Close sorin-ionescu#140 PR: Add git arrows customization. Fixes sorin-ionescu#139.
  > 1.1.1
  > Disable prompt expansion for running command
  > Merge pull request sorin-ionescu#130 from zmwangx/rename-variables-for-readability
  > Use standard `[[ ]]` for conditional and add clarifications
  > fix: do the PURE_GIT_PULL check in the correct place
  > 1.1.0
  > Close sorin-ionescu#124 PR: Show hostname in terminal title if session is over ssh.
  > Merge pull request sorin-ionescu#125 from sindresorhus/dirty-check
  > import bug-fix release from zsh-async, fixes async job flushing
  > readme: faq clarificaitons
  > readme: add zpty error to faq with explanation and potential solutions
  > readme: update instructions for antigen and oh-my-zsh. remove incompatible async.plugin.zsh
  > use cd -q to prevent hooks from firing
  > prevent git status leakage when testing if dirty
  > fix paths that are split due to spaces in directory names
kodelint pushed a commit to kodelint/prezto that referenced this pull request Nov 15, 2016
RIT80 pushed a commit to RIT80/prezto that referenced this pull request Jan 25, 2022
Functions cdll, pushdll, and popdll depend on the ll alias.
RIT80 pushed a commit to RIT80/prezto that referenced this pull request Jan 25, 2022
RIT80 pushed a commit to RIT80/prezto that referenced this pull request Jan 25, 2022
RIT80 pushed a commit to RIT80/prezto that referenced this pull request Jan 25, 2022
RIT80 pushed a commit to RIT80/prezto that referenced this pull request Jan 25, 2022
RIT80 added a commit to RIT80/prezto that referenced this pull request Jan 25, 2022
Functions cdll, pushdll, and popdll depend on the ll alias.
RIT80 added a commit to RIT80/prezto that referenced this pull request Jan 25, 2022
RIT80 added a commit to RIT80/prezto that referenced this pull request Jan 25, 2022
RIT80 added a commit to RIT80/prezto that referenced this pull request Jan 25, 2022
RIT80 added a commit to RIT80/prezto that referenced this pull request Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants