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

Handling of invalid values in the Search, Search, Replace and Replace methods #370

Merged
merged 2 commits into from
Jan 4, 2025

Conversation

Ekopalypse
Copy link
Contributor

fixes #172

Additional methods such as editor.replaceSel must be handled differently, as these are automatically generated by the iface file.

@chcg
Copy link
Collaborator

chcg commented Jan 3, 2025

@Ekopalypse The original "request" was:

I was semi-surprised to see a non-zero result printed.
Any chance that the plugin could trap on this situation and not match anything?

From that point I think your PR is a different approach and notifies the user about "invalid" or "problematic" input.

@Ekopalypse
Copy link
Contributor Author

Ekopalypse commented Jan 4, 2025

@chcg, @sasumner
So PS should return an empty list instead?
What would be the use case for searching for an empty string?

@alankilborn
Copy link

Well, nobody asked me, but:

  • I suppose an empty list would be fine
  • There is no use case for intentionally searching for an empty string (but it could happen unintentionally)

@Ekopalypse
Copy link
Contributor Author

... but it could happen unintentionally

but the code logic is not aware of this.
In the case of an empty list, this could mean that nothing was found instead of nothing having been searched for. Wouldn't it make more sense to return that nothing was searched for?

@alankilborn
Copy link

Wouldn't it make more sense to return that nothing was searched for?

Ah, I see.
The problem is in how to return that.
It appears the PR code creates an exception...

@Ekopalypse
Copy link
Contributor Author

It appears the PR code creates an exception...

Correct, this PR aims to prohibit empty strings as search arguments and None as search or replace arguments.

@alankilborn
Copy link

So, my choices will be, that every search I do after this PR is integrated will need to look like:

if len(find_text) == 0:
    ...
else:
    editor.research(find_text, ...)

or:

try:
    editor.research(...)
except:
    ...

because we can't have the script user seeing the exception/traceback.

I'm just trying to understand my options. :-)

@Ekopalypse
Copy link
Contributor Author

I don't understand the question. You can still use editor.research(find_text, ...) without the try or if block, as long as you know the input is valid.
If you're writing a script where someone else is doing the input, you need to check the input anyway, since it's probably based on certain predefined rules, right?
The question that needs to be answered from my point of view is, do we return an empty list with the risk of giving the impression that something was not found even though nothing was searched for or do we throw an exception if the arguments that are expected are empty string or None.

@alankilborn
Copy link

I don't understand the question.

No worries; it was simply me rambling. :-)


do we return an empty list with the risk of giving the impression that something was not found even though nothing was searched for

No.

or do we throw an exception if the arguments that are expected are empty string or None.

Yes.

@chcg
Copy link
Collaborator

chcg commented Jan 4, 2025

@Ekopalypse @alankilborn So we start to go for the exception solution until a reasonable complain is coming up.

@chcg chcg merged commit cb29713 into bruderstein:master Jan 4, 2025
6 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.

Prevent "research" from generating matches if search string is zero-length
3 participants